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

Use select to query the uv kqueue #5378

Merged
merged 1 commit into from May 4, 2016

Conversation

Projects
None yet
2 participants
@CGamesPlay
Copy link
Contributor

CGamesPlay commented May 3, 2016

This resolves #38. Read the issue for context, but after debugging the problem looks like this:

When a pty is in the uv kqueue, the poll kqueue does not properly clear the event after receiving it. The obvious fix would be to add EV_CLEAR to the event flags so that it is definitely cleared after receiving any event, however this causes a race condition in Darwin. The poll kqueue is signaled before the uv kqueue, resulting in the uv thread running once, seeing no events, and going back to sleep, while simultaneously the poll thread clears its kqueue out. This results in events being delayed until the next event comes in. You can verify that this is a race condition by adding EV_CLEAR to the original kevent call and then adding a sleep at the end of PollEvents

The code is simply waiting for data to be available for reading on the uv kqueue file descriptor, which select also does. I have migrated the Darwin-specific uv polling code to use select instead of kqueue.

I've verified that events still get processed like they should on El Capitan 10.11.3 (15D21).

Note: poll doesn't work on kqueues, otherwise I would have used that instead.

Use select to query the uv kqueue
This resolves #38. I've verified that events still get processed like they
should on El Capitan 10.11.3 (15D21).
@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented May 4, 2016

This is clever, let's try it! 👍

@zcbenz zcbenz merged commit ef561fb into electron:master May 4, 2016

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.