search

pexpect / ptyprocess

Run a subprocess in a pseudo terminal
https://ptyprocess.readthedocs.io/en/latest/
Other
222 stars 71 forks source link

Rather than closing all the file descriptors, set close on exec on them #51

Open petesh opened 5 years ago

petesh commented 5 years ago

The code that closes all the file descriptors in the child process before exec breaks practically any and all efforts to debug exec errors; especially if they're not OSErrors.

In my case, I was debugging under pycharm. pydevd monkeypatched all the exec routines and because of a behavioural change in pexpect, where it re-encoded all the arguments as utf-8 encoded binary, rather than strings, when pydevd tried to detect the presence of python in it's code, it crashed out because there was encoding of the strings and bstring.endswith() needs to be passed a b'' string, or else it bombs out.

petesh commented 5 years ago

currently linux only solution: https://github.com/petesh/ptyprocess/commit/a4a0090284acb56207e49d6ea58c4a5b8f56dded I'll see about other solutions

takluyver commented 5 years ago

The Python subprocess module also appears to close fds explicitly rather than setting CLOEXEC, although it happens in C code rather than Python. I'd want to hear from the maintainers of that whether there are any downsides to setting CLOEXEC before changing that.

petesh commented 5 years ago

Because it happens in python, it still leaves the cleanup of objects prior to the exec; which has hidden swallowed OSError exceptions coming from instances of classes being torn down who have file.close() calls in their __del__ methods.

It's not really safe code. CLOEXEC is a somewhat better solution as your code path is trying to get to only there, and if it fails, you always exit.