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

src/usocket: Do not setblocking on destroy; #108

Merged
merged 2 commits into from Oct 27, 2014

Conversation

daurnimator
Copy link
Contributor

This results in unexpected behaviour if the socket has been dup()d, as O_NONBLOCK is shared.
Close is always 'blocking' anyway (on unix systems)

See wahern/cqueues#13 for an example use case

I have not done the same for windows (src/wsocket.c) as closesocket() can have non-blocking behaviour.
see http://msdn.microsoft.com/en-us/library/windows/desktop/ms737582(v=vs.85).aspx

This results in unexpected behaviour if the socket has been `dup()`d, as O_NONBLOCK is shared.
Close is always 'blocking' anyway

See wahern/cqueues#13 for an example use case
@daurnimator
Copy link
Contributor Author

@diegonehab to answer your earlier question about shutdown(), that also has no need to call setblocking(), as it does not have a non-blocking mode (on unix).

I can add that to this Pull Request if you want?

@diegonehab
Copy link
Contributor

Please add to the shutdown modification to the pull request?

… calls.

It doesn't effect them.
Not true on windows
@daurnimator
Copy link
Contributor Author

^^ done. also for listen() which also doesn't care about O_NONBLOCK.

I'm not convinced that was necessary; though it can't harm on the systems I looked at.
destroy was a special case, because it was impossible to avoid (as it's the __gc), and there is no other close() function available.

diegonehab added a commit that referenced this pull request Oct 27, 2014
src/usocket: Do not setblocking on destroy, shutdown, and listen.
@diegonehab diegonehab merged commit 6dcecd8 into lunarmodules:master Oct 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants