Skip to content

Make sure UDP send callbacks are always fired asynchronously #371

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.

Sign up for GitHub

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

Jump to bottom
Merged
saghul merged 2 commits into from
May 28, 2015
Merged

Make sure UDP send callbacks are always fired asynchronously #371

saghul merged 2 commits into from
May 28, 2015

Conversation

@saghul
Copy link
Member

@saghul saghul commented May 27, 2015

R=@bnoordhuis

Fixes: #301

@saghul
Copy link
Member Author

saghul commented May 27, 2015

Well, fuck.I'm afraid the change to uv__run_pending has some interesting side effects, as in 40 failing tests or something :-(

@saghul
Copy link
Member Author

saghul commented May 27, 2015

The problem seems to be the following:

  • The first time around, uv_udp_send sends immediately, so uv__udp_sendmsg is called and the io handle is set as pending with uv__io_feed, in order to run the callback on the next tick.
  • On the next tick, while in uv__run_pending, we call uv_udp_send again, and because we get UV__POLLOUT, we also try to write the queued send requests, and we succeed! so we call uv__io_feed again, to call its callback.
  • Since we're ietarting the pending queue, and uv__io_feed adds the handle to that very same queue, we enter an infinite loop.

I'll keep digging...

@saghul
Copy link
Member Author

saghul commented May 27, 2015

Solved it! All tests pass, it was a fuckup in using QUEUE_FOREACH. @bnoordhuis can you PTAL?

@bnoordhuis
Copy link
Member

bnoordhuis commented May 27, 2015

LGTM

@saghul saghul merged commit 1816dbc into libuv:v1.x May 28, 2015
@saghul
Copy link
Member Author

saghul commented May 28, 2015

Thanks Ben! Adjusted the comment and landed.

@saghul saghul mentioned this pull request May 28, 2015
Closed
saghul added 2 commits May 28, 2015 11:05
@saghul
unix: prevent infinite loop in uv__run_pending
3ceb260 
If any of the callbacks called in uv__run_pending calls
uv__io_feed we enter an infinite loop, because we add the handle to the
same queue we are iterating over.

Refs: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
@saghul
unix: make sure UDP send callbacks are asynchronous
1816dbc 
Fixes: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
saghul added a commit to saghul/node that referenced this pull request Jun 3, 2015
@saghul
dgram: partially revert 18d457b
7456692 
Revert "dgram: call send callback asynchronously" partially, since the
fix is now done in libuv.

Refs: nodejs#1313
Refs: libuv/libuv#371
bnoordhuis pushed a commit to nodejs/node that referenced this pull request Jun 3, 2015
@saghul @bnoordhuis
dgram: partially revert 18d457b
b5cd2f0 
Revert "dgram: call send callback asynchronously" partially, since the
fix is now done in libuv.

Refs: #1313
Refs: libuv/libuv#371
PR-URL: #1889
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
guworks pushed a commit to guworks/libuv that referenced this pull request Jul 9, 2015
@saghul @guworks
unix: prevent infinite loop in uv__run_pending
331218e 
If any of the callbacks called in uv__run_pending calls
uv__io_feed we enter an infinite loop, because we add the handle to the
same queue we are iterating over.

Refs: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
guworks pushed a commit to guworks/libuv that referenced this pull request Jul 9, 2015
@saghul @guworks
unix: make sure UDP send callbacks are asynchronous
efcafdf 
Fixes: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
skomski pushed a commit to skomski/libuv that referenced this pull request Aug 5, 2015
@saghul @skomski
unix: prevent infinite loop in uv__run_pending
0a5cd56 
If any of the callbacks called in uv__run_pending calls
uv__io_feed we enter an infinite loop, because we add the handle to the
same queue we are iterating over.

Refs: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
skomski pushed a commit to skomski/libuv that referenced this pull request Aug 5, 2015
@saghul @skomski
unix: make sure UDP send callbacks are asynchronous
5c12f7c 
Fixes: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
skomski pushed a commit to skomski/libuv that referenced this pull request Aug 5, 2015
@saghul @skomski
unix: prevent infinite loop in uv__run_pending
a4e738a 
If any of the callbacks called in uv__run_pending calls
uv__io_feed we enter an infinite loop, because we add the handle to the
same queue we are iterating over.

Refs: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
skomski pushed a commit to skomski/libuv that referenced this pull request Aug 5, 2015
@saghul @skomski
unix: make sure UDP send callbacks are asynchronous
9d030ae 
Fixes: libuv#301
PR-URL: libuv#371
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

udp: callbacks are effectively synchronous

2 participants

@saghul @bnoordhuis