[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
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

Fix bpo-30589: improve Process.exitcode with forkserver #1989

Merged
merged 3 commits into from Jun 12, 2017

Conversation

Copy link
Member

pitrou commented Jun 8, 2017

When the child is killed, Process.exitcode should return -signum, not 255.

When the child is killed, Process.exitcode should return -signum, not 255.
Copy link

mention-bot commented Jun 8, 2017

@pitrou, thanks for your PR! By analyzing the history of the files in this pull request, we identified @shibturn, @benjaminp and @jnoller to be potential reviewers.

Copy link
Member Author

pitrou commented Jun 8, 2017

@cf-natali

pitrou added the type-feature A feature request or enhancement label Jun 8, 2017
Copy link
Member Author

pitrou commented Jun 8, 2017

@cf-natali, do you know if the SIGCHLD strategy used here is reliable?

Copy link
Contributor

cf-natali commented Jun 9, 2017

pitrou force-pushed the forkserver_child_signal branch from bb62a7f to e30cdc4 Compare Jun 12, 2017
Copy link
Member Author

pitrou commented Jun 12, 2017

Thanks for the review @cf-natali. There are other places in forkserver where spurious notifications are not considered, so I'm gonna judge that the sig_r pipe doesn't warrant special care.

pitrou merged commit dfd5f34 into python:master Jun 12, 2017
pitrou deleted the forkserver_child_signal branch Jun 12, 2017
Copy link
Member Author

pitrou commented Jun 12, 2017

I've determined that this PR suffers from a race condition (a well-known one, admittedly): SIGCHLD can arrive just before epoll_wait is called, in which case the latter doesn't return EINTR and our event loop just sits waiting for further events. Then some forkserver-based test randomly hangs (seen in some CI builds).

The solution seems to be to use signal.set_wakeup_fd.

pitrou added a commit to pitrou/cpython that referenced this pull request Jun 12, 2017
…hon#1989)

There's an admittedly well-known race condition where ECHILD can arrive
just before the C function epoll_wait() and the latter wouldn't therefore
return EINTR.  The solution is to use set_wakeup_fd(), which was designed
to avoid such race conditions.
pitrou added a commit that referenced this pull request Jun 13, 2017
…p to PR #1989) (#2139)

* Fix race condition in signal wakeup in forkserver (followup to PR #1989)

There's an admittedly well-known race condition where ECHILD can arrive
just before the C function epoll_wait() and the latter wouldn't therefore
return EINTR.  The solution is to use set_wakeup_fd(), which was designed
to avoid such race conditions.

* Reset wakeup fd in child
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants