Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

signal: ignore tty.resize errors #1575

Merged
merged 1 commit into from Sep 1, 2017
Merged

signal: ignore tty.resize errors #1575

merged 1 commit into from Sep 1, 2017

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Aug 26, 2017

Fixes a race that occurred very frequently in testing where the tty of
the container may be closed by the time that runc gets to sending
SIGWINCH. This failure mode is not fatal, but it would cause test
failures due to expected outputs not matching. On further review it
appears that the original addition of these checks in 4c5bf64
("Check error return values") was actually not necessary, so partially
revert that change.

The particular failure mode this resolves would manifest as error logs
of the form:

time="2017-08-24T07:59:50Z" level=error msg="bad file descriptor"

Fixes: 4c5bf64 ("Check error return values")
Fixes #1571
Signed-off-by: Aleksa Sarai asarai@suse.de

Fixes a race that occurred very frequently in testing where the tty of
the container may be closed by the time that runc gets to sending
SIGWINCH. This failure mode is not fatal, but it would cause test
failures due to expected outputs not matching. On further review it
appears that the original addition of these checks in 4c5bf64
("Check error return values") was actually not necessary, so partially
revert that change.

The particular failure mode this resolves would manifest as error logs
of the form:

  time="2017-08-24T07:59:50Z" level=error msg="bad file descriptor"

Fixes: 4c5bf64 ("Check error return values")
Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar
Copy link
Member Author

cyphar commented Aug 26, 2017

/cc @tklauser Does this make sense to you? Sorry for not catching this race earlier.

Maybe we should just check for EBADF but Go's error "cause" handling isn't very good. 🙁

@tklauser
Copy link
Contributor

Thanks @cyphar! Didnt't consider this possibility when submitting the original patch. Your fix LGTM.

@crosbymichael
Copy link
Member

crosbymichael commented Aug 28, 2017

LGTM

Approved with PullApprove

1 similar comment
@mrunalp
Copy link
Contributor

mrunalp commented Sep 1, 2017

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit aea4f21 into opencontainers:master Sep 1, 2017
@cyphar cyphar deleted the tty-resize-ignore-errors branch September 2, 2017 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exec: "bad file descriptor" occurs in tests
4 participants