Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
petesh opened this issue Feb 26, 2019 · 3 comments
Open

Comments

@petesh
Copy link

petesh commented Feb 26, 2019

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 added a commit to petesh/ptyprocess that referenced this issue Feb 26, 2019
@petesh
Copy link
Author

petesh commented Feb 26, 2019

currently linux only solution: petesh@a4a0090
I'll see about other solutions

@takluyver
Copy link
Member

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
Copy link
Author

petesh commented Feb 27, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants