Skip to content

Conversation

@openshift-cherrypick-robot

This is an automated cherry-pick of #4354

/assign fidencio

None

This patch fixes the situation of `kubectl cp` hanging forever.

Debugging this issue I've faced a quite interesting situatuon where
after `closeIOChan` is closed, the `CloseStdin` functions "resumes"
while `closeIOChan` is **nil**, making the function to sit there and
wait forever.

Honestly, this bugs my mind quite a bit as I was under the impression
that all the everyone waiting for the channel to be closed would be
notified, would consume its content, and only after that it'd be
nullified.  Well, Today I learnt.

The simplest and more future proof way to work this issue around is to
ensure `closeIOChan` is not **nil** inside CloseStdin's function.

Thanks to @liubin, @haircommander, and @r4f4 for being available to
discuss the issue.

Fixes: cri-o#4353

Signed-off-by: Fabiano Fidêncio <[email protected]>
@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #4360 (169408d) into release-1.18 (aba91e5) will not change coverage.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##           release-1.18    #4360   +/-   ##
=============================================
  Coverage         38.69%   38.69%           
=============================================
  Files               106      106           
  Lines              7941     7941           
=============================================
  Hits               3073     3073           
  Misses             4569     4569           
  Partials            299      299           

@fidencio
Copy link
Contributor

fidencio commented Nov 9, 2020

/retest

2 similar comments
@fidencio
Copy link
Contributor

/retest

@fidencio
Copy link
Contributor

/retest

@TomSweeneyRedHat
Copy link
Contributor

LGTM
Assuming happy test

@umohnani8
Copy link
Member

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: openshift-cherrypick-robot, umohnani8

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2020
@openshift-merge-robot openshift-merge-robot merged commit ee91284 into cri-o:release-1.18 Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants