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

Better handling of os.fork from inside trio #464

Open
njsmith opened this issue Mar 4, 2018 · 4 comments
Open

Better handling of os.fork from inside trio #464

njsmith opened this issue Mar 4, 2018 · 4 comments

Comments

@njsmith
Copy link
Member

njsmith commented Mar 4, 2018

If you call os.fork from inside a trio context, and then try to use trio in both the parent and the child, you are going to have a bad time. fork is a very powerful tool, and leaves the process in a strange state: the child is just like the parent, except that threads etc. have disappeared, and they share all fd's (including the self-pipe), etc. This seems essentially unsolvable: if you want to start a new process from trio using fork+exec, that's fine, but anything else is probably not going to work. I guess if you fork and then only execute synchronous code in the child then things might be mostly OK (though you've just leaked a bunch of memory in the child, because it will forever keep copies of all the parent process's data, and copy-on-write won't save you because of Python's well-known issues with GC causing un-sharing).

(See also bpo-21998, which is asyncio's version of this issue.)

We should at least document this. It would also be good to actually catch it and provide some error, since nobody reads the documentation.

One approach is to record the pid when we enter trio, and then each time we touch trio state, verify that os.getpid() still gives the same value. I guess "enter trio" here means any of the @_public methods in trio/_core, yielding to the event loop, and TrioToken.run_sync_soon?

In 3.7 there's os.register_at_fork, which could potentially be useful (maybe to reduce overhead?), but I'm not entirely sure how.

@smurfix
Copy link
Contributor

smurfix commented Mar 10, 2018

I don't particularly like fiddling with getpid() all the time, that's 99.999999% nonproductive work trying to catch the one mistake somebody made. I can't think of a way to get a worse usefulness/required_work ratio than that.

IMHO the best solution would be to os.register_at_fork an after_in_child handler which monkeypatch-poisons the current Trio mainloop and/or task so that it'll fall flat on its face the next time it's entered, preferably in such a way that there's a semi-understandable error message. That at least would cause no additional busywork for the common case (no-fork-ever).

@njsmith
Copy link
Member Author

njsmith commented Mar 10, 2018 via email

@smurfix
Copy link
Contributor

smurfix commented Mar 10, 2018

At the very least, checking a flag for True/False is less expensive than a system call.

Another way would be to simply del GLOBAL_RUN_CONTEXT.task which would then trigger an internal Trio error when run_impl() tries to do the same thing, if not sooner.

@sseg
Copy link

sseg commented Mar 15, 2018

Would that be sufficient to raise a warning on fork? Since the first-order solution was documentation, it seems unnecessary to me to do more than document the undefined behavior at runtime.

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

No branches or pull requests

4 participants