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 SleepEx instead of Sleep #47

Merged
merged 1 commit into from
Sep 26, 2015
Merged

Conversation

nurse
Copy link
Contributor

@nurse nurse commented Sep 26, 2015

Sleep is replaced with rb_w32_Sleep by Ruby,
which tries to release GVL, though select_poll
doesn't have GVL.

Sleep is replaced with rb_w32_Sleep by Ruby,
which tries to release GVL, though select_poll
doesn't have GVL.
@@ -193,26 +191,6 @@ select_poll (EV_P_ ev_tstamp timeout)
int res;
int fd_setsize;

#ifdef _WIN32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does using SleepEx care this situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once I thought NULL fds for select cause stuck, but https://github.com/tarcieri/cool.io/pull/47/files#diff-c9164d22260cba57b9d4a9beb08b42d0L258 says it will return EINVAL. I confirmed it with simple C program.
Now I think I misunderstood stuck on select() instead of stuck on Sleep().

Anyway this code still has a meaning as shortcut (skip select() and just sleep), but I choose code size.

repeatedly added a commit that referenced this pull request Sep 26, 2015
@repeatedly repeatedly merged commit eeb9f48 into socketry:master Sep 26, 2015
@repeatedly
Copy link
Contributor

Okay. Just merged.

@nurse nurse deleted the windows-select-fix branch September 26, 2015 07:11
nurse added a commit to fluent/fluentd that referenced this pull request Sep 28, 2015
nurse added a commit to fluent/fluentd that referenced this pull request Sep 29, 2015
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

Successfully merging this pull request may close these issues.

2 participants