mitogen/parent: first stage: Bail out if stdin/stdout is not accessible#1395
mitogen/parent: first stage: Bail out if stdin/stdout is not accessible#1395mhartmay wants to merge 3 commits into
Conversation
Recommend you wait until I've also done a release. |
mhartmay
force-pushed
the
bail-out-stdin-stdout
branch
2 times, most recently
from
0b33371 to
840b9ed
Compare
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Bail out if STDIN or STDOUT is closed or unavailable, as these streams are required for the communication with the parent process. Without this check, the later `os.pipe()` calls in the first_stage may return file descriptors 0 and 1, leading to a confusing and hard-to-diagnose situation. -SSH command size: 838 +SSH command size: 850 -mitogen.parent 99240 96.9KiB 51244 50.0KiB 51.6% 12956 12.7KiB 13.1% +mitogen.parent 99496 97.2KiB 51275 50.1KiB 51.5% 12964 12.7KiB 13.0% Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
mhartmay
force-pushed
the
bail-out-stdin-stdout
branch
from
840b9ed to
0efd2c3
Compare
Done. |
|
Thank you, I'll look at this over Christmas. |
| # Bail out in case STDIN or STDOUT is not accessible (e.g. closed). | ||
| # Otherwise, os.pipe() could reuse file descriptors 0 or 1, leading to | ||
| # unexpected behavior that is difficult to diagnose. |
There was a problem hiding this comment.
reuse file descriptors 0 or 1, leading to unexpected behavior that is difficult to diagnose.
Note to self: If this occured it would be more acute than I realised. This assumption - that fd 0, 1, 2 are stdio - is made/relied just a few lines down, in the first stage. Let alone elsewhere in Mitogen, or other code bases.
There was a problem hiding this comment.
Open (to me) questions:
- Can it occur? Could fd 0 (stdin) and/or fd 1 (stdout) be closed at theis point?
- Has it occured?
There was a problem hiding this comment.
It can be constructed (see my test cases), if those cases are relevant - I'm not sure. And I've checked the FDs by writing the FD numbers to a file (as stdout/stderr is not available).
| @@ -1455,6 +1459,10 @@ def _first_stage(): | |||
| f.write(C) | |||
| f.close() | |||
| os.write(1,'MITO001\n'.encode()) | |||
There was a problem hiding this comment.
Pre-existing: Assumes fd 1 is stdout
| # stream with "[1234 refs]" during exit. | ||
| # If STDERR is already closed an OSError is raised, but no one cares | ||
| # as STDERR is closed and the exit status is not forwarded. | ||
| os.close(2) |
There was a problem hiding this comment.
Pre-existing: Assumes fd 1 is stderr
2 participants
Bail out if STDIN or STDOUT is closed or unavailable, as these streams are
required for the communication with the parent process. Without this check, the
later
os.pipe()calls in the first stage may return file descriptors 0 and 1,leading to a confusing and hard-to-diagnose situation.