linux: always deregister closing fds from epoll #1100
Conversation
|
Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion. The following commiters were not found in the CLA:
You can fix all these things without opening another issue. Please see CONTRIBUTING.md for more information |
| * | ||
| * We pass in a dummy epoll_event, to work around a bug in old kernels. | ||
| */ | ||
| if (loop->backend_fd >= 0) { |
|
Thank you! LGTM, but it really needs to have a test. Just FYI, the relevant files to introduce a new test:
|
|
@goffrie just thought that you may find this useful for implementing test: https://gist.github.com/indutny/8842007 . Anyway, if you don't feel like you want or have time to do it, let me know. |
If the same file description is open in two different processes, then closing the file descriptor is not sufficient to deregister it from the epoll instance (as described in epoll(7)), resulting in spurious events that cause the event loop to spin repeatedly. So always explicitly deregister it. Fixes #1099.
|
Alright, I fixed a little issue ( |
|
|
||
| TEST_IMPL(closed_fd_events) { | ||
| /* spawn_helper4 blocks indefinitely. */ | ||
| char *argv[] = { NULL, "spawn_helper4", NULL }; |
There was a problem hiding this comment.
char* argv, not char *argv, please
|
Looks awesome! Thank you very much! Some style issue, but generally LGTM. I hope you'll agree with me that it would be simpler to have it in |
|
Alright, I've moved the test into |
|
Sorry for the delay -- I'm waiting for my employer to get back to me. |
|
Hey man! Any news? |
|
I've signed the CLA now. :) |
|
Thank you, landed in 780d8ad |
If the same file description is open in two different processes, then
closing the file descriptor is not sufficient to deregister it from the
epoll instance (as described in epoll(7)), resulting in spurious events
that cause the event loop to spin repeatedly. So always explicitly
deregister it.
Fixes #1099.