Removed the SIGWINCH race and added a check for EINTR in write_all() #18

wants to merge 1 commit into


None yet
2 participants

robryk commented Feb 27, 2012

I think this removes the problem from FIXME. A check in write_all() seems worth it,
because those writes may very well block.

By the way, the fork-and-wait-for-syscall-to-change method may break something
if the program has any shared read-write memory mappings and the child
happens to write to them between fork and next syscall. I don't see an easy way around
this, though (moving IP to the private page only comes to my mind). That said,
it's probably a very rare scenario.

This doesn't actually fix the interesting race condition. The tricky race is the one that happens if SIGWINCH happens after this point, but before we enter select(). In that case, we won't process the resize until we see further input on one side of the pty.

This patch does help things slightly, in that I believe it should mean we can't drop a SIGWINCH completely, which is an improvement.


nelhage commented Feb 28, 2012

Merged, with some slight modifications to be more pedantic about the volatile variable, and document the remaining race.

@nelhage nelhage closed this Feb 28, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment