This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: pythorun.c: is_valid_fd() should not duplicate the file descriptor
Type: Stage:
Components: Versions: Python 3.4
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: pitrou, tshepang, vstinner
Priority: normal Keywords:

Created on 2013-08-21 23:07 by vstinner, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (8)
msg195836 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-08-21 23:07
is_valid_fd() of Python/pythorun.c should use fstat() to check if a file descriptor is valid, instead of duplicating it using dup() (and then closing it).

If Windows needs a special check, it would be better to run checks on the handle of the file descriptor.

The function was added by the following changeset:

----------------------------
branch:      3.2
parent:      73773:661fb211f220
user:        Antoine Pitrou <solipsis@pitrou.net>
date:        Mon Nov 28 19:08:36 2011 +0100
files:       Lib/test/test_cmd_line.py Misc/NEWS Python/pythonrun.c
description:
Issue #7111: Python can now be run without a stdin, stdout or stderr stream.

It was already the case with Python 2.  However, the corresponding
sys module entries are now set to None (instead of an unusable file object).
----------------------------


See also issue #17797 "Visual C++ 11.0 reports fileno(stdin) == 0 for non-console program".
msg195837 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-21 23:08
> is_valid_fd() of Python/pythorun.c should use fstat() to check if a
> file descriptor is valid, instead of duplicating it using dup() (and
> then closing it).

Why should it?
fstat() may be expensive, while dup() is cheap.
msg195838 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-08-21 23:24
> Why should it?

fstat() does not need to create a new file descriptor. Creating a new file descriptor can fail (ex: limit of the number of open files).
msg195839 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-21 23:26
> > Why should it?
> 
> fstat() does not need to create a new file descriptor. Creating a new
> file descriptor can fail (ex: limit of the number of open files).

Why should I care? fstat() can fail too.
The only important thing is whether dup() returns EBADF or not.
msg195840 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-21 23:28
By the way, can you please mention the actual changeset id? (not only the parent)
msg195841 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-08-21 23:33
> By the way, can you please mention the actual changeset id? (not only the parent)

Oh, I failed to copy/paste the changeset, sorry :-) It is the changeset f15943505db0:

changeset:   73775:f15943505db0
branch:      3.2
parent:      73773:661fb211f220
user:        Antoine Pitrou <solipsis@pitrou.net>
date:        Mon Nov 28 19:08:36 2011 +0100
files:       Lib/test/test_cmd_line.py Misc/NEWS Python/pythonrun.c
description:
Issue #7111: Python can now be run without a stdin, stdout or stderr stream.

It was already the case with Python 2.  However, the corresponding
sys module entries are now set to None (instead of an unusable file object).
msg195842 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-21 23:34
Thanks. I guess is_valid_fd() could be improved to check for EBADF, but other than that I don't see the problem. dup() is better than fstat() which may do some actual I/O.
Did you encounter this in real life?
msg199568 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-10-12 13:20
> Did you encounter this in real life?

Well, my initial concern was that dup() creates an inheritable file descriptor. It is unlikely that fork() occurs while is_valid_fd() is called, because is_valid_fd() is only called early during Python initialization.

Replacing dup() with _Py_dup() would be overkill: _Py_dup() releases the GIL and raises an exception, which is not needed here.

I'm closing the issue. I will reopen it if I find a simple solution to this non-issue :-)
History
Date User Action Args
2022-04-11 14:57:49adminsetgithub: 63004
2013-10-12 13:20:58vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg199568
2013-08-23 16:54:10tshepangsetnosy: + tshepang
2013-08-21 23:34:53pitrousetmessages: + msg195842
2013-08-21 23:33:21vstinnersetmessages: + msg195841
2013-08-21 23:28:45pitrousetmessages: + msg195840
2013-08-21 23:26:35pitrousetmessages: + msg195839
2013-08-21 23:24:14vstinnersetmessages: + msg195838
2013-08-21 23:08:58pitrousetmessages: + msg195837
2013-08-21 23:07:47vstinnercreate