homepage

Looks like this bug https://bugs.python.org/issue7111 has resurfaced in python3 (python 2.6 works as far as I can tell) at least on Macs.
I've attached a simple test script.
Steps:
1. SSH to remote server.
2. Run nohup ./test.sh &
3. exit SSH.
4. SSH back in and see that there are a bunch of errors in your nohup.out file.

test.sh attached

What sort of errors?

I'm unable to reproduce the issue on Linux with Python 3.4.2.

Fatal Python error: Py_Initialize: can't initialize sys standard streams
OSError: [Errno 9] Bad file descriptor
./test.sh: line 4: 49632 Abort trap: 6 python3 test.py -o hello.txt

That's from my nohup.out. It might be a Mac OS specific thing.

I can reproduce the bug with gdb if the file descriptor 0 is closed before calling:
 std = create_stdio(iomod, fd, 0, "<stdin>", encoding, errors);
and after the following lines were called:
 fd = fileno(stdin);
 if (!is_valid_fd(fd)) {
In initstdio () at Python/pylifecycle.c:1156.
create_stdio() fails in FileIO constructor, on the line:
 if (_Py_fstat(self->fd, &fdfstat) < 0)
of _io_FileIO___init___impl() at Modules/_io/fileio.c:480
It's a corner case of the issue #7111, I would call it a race condition.

Python 2 is not affected, PyFile_FromFile() doesn't check if the file descriptor is valid. On Python 3, we call fstat() to check the block size of the file descriptor to optimize buffering.

I'm not sure this is a race condition. There's a crash every single time python is started when running the test.sh script under nohup.
If it's a race condition, it should only happen some of the time, not every single time.
Can you reeevaluate the priority?

Attaching a patch. Does it make any sense?

ops wrong patch... trying again.

Yes, it looks like a good approach. Some comments:
- please mention this issue "Issue #24891" and explain the issue in a
comment in create_stdio_checked()
- it would be safer to check the exception type, to be extra safe
- you must clear the exception
- why not putting the code in create_stdio() directly?

@haypo thanks for the quick review. This new issue24891_2.patch covers all of the points you raised except the "check exception type" which I am still figuring out.

> This new issue24891_2.patch covers all of the points you raised except the "check exception type" which I am still figuring out.
See my patch issue24891_3.patch which calls PyErr_ExceptionMatches(PyExc_OSError). If you like it, I can push it to Python 3.4-3.6.

@haypo, yeah, definitely better than mine! All good for me.

New changeset e67bf9c9a898 by Victor Stinner in branch '3.4':
Fix race condition in create_stdio()
https://hg.python.org/cpython/rev/e67bf9c9a898 

> @haypo, yeah, definitely better than mine! All good for me.
Ok. I added your name to Misc/ACKS. I had to do some tricks to apply the patch to Python 3.4 (code was in Python/pythonrun.c) and then to merge to Python 3.5 (code moved to Python/pylifecycle.c). But I checked the fix in each version using gdb:
* put a breakpoint on create_stdio,
* type "print close(0)" after the first is_valid_fd() check,
* see that the open() exception is correctly handled (type "next", "next", ... and check that we go to the error: label and then enter the if() block)
Thanks for your patch Marco.
I prefer to be extra safe by checking the raised exception to minimize the risk of regression.

Thank you everyone! I will test the next rc of 3.5.

I don't plan to send a pull request to the Python 3.5 release manager.
This bug existed since Python 3.0, the fix can wait for Python 3.5.1.