| marmoute |
| hg-reviewers |
On Python 3 stdio objects can be None. That causes crashes. Fix it by opening
devnull automatically.
stdin can be None by using 0<&- in bash, or spawning processes less
carefully, for example, watchman used to cause such None stdin [1] (note:
None is only observable on Python 3).
[1]: https://github.com/facebook/watchman/commit/d241978aaa6b6d7c5b7260bc9e6d699d3a1cea53
| No Linters Available |
| No Unit Test Coverage |
I think it’s reasonable that Mercurial expects that the stdio streams are initialized properly. Is there a use case for running Mercurial with FDs 0, 1 or 2 set to non-standard streams?
| mercurial/utils/procutil.py | ||
|---|---|---|
| 119–121 | The created streams are replaced later by initstdio(), causing them to get garbage-collected without getting closed explicitly. If you set environment variable PYTHONWARNINGS=always, you’ll get something like /home/manu/vcs/hg/mercurial/dispatch.py:189: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='r' encoding='UTF-8'> sys.stdin = io.TextIOWrapper( | |
| 121 | Is there a reason for not opening a dedicated stream for stderr? | |
This patch breaks my brain because our mucking about with sys.std* is wonky. We are doing really hacky things in both procutil.py and dispatch.py, including reassigning sys.std* in dispatch.py before this patch. I'd strongly prefer to have that logic consolidated.
As for the Python warning, that's super annoying. But I'm not sure we can do anything about that since we don't have an obvious place to insert an __exit__ handler. This is untested, but the best I can come up with is:
if not sys.stdin:
sys.stdin = open(os.devnull, 'r')
atexit.register(sys.stdin.close)As Pulkit pointed out, I and Pulkit made another version in parallel.
https://patchwork.mercurial-scm.org/patch/47937/
https://patchwork.mercurial-scm.org/patch/47938/
https://patchwork.mercurial-scm.org/patch/47939/
sys.std* are left unmodified. procutil.std* are fixed up to raise EBADF.