Skip to content

Conversation

santigimeno
Copy link
Member

Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a setImmediate callback has been called
in-between. It avoids a race condition in the test where the recursive
limit is reached without having received at least 10 messages.

I was sometimes getting this error in OS X:

/Users/sgimeno/node/node/test/parallel/test-dgram-send-callback-recursive.js:15
    throw new Error('infinite loop detected');
    ^

Error: infinite loop detected
    at SendWrap.onsend [as callback] (/Users/sgimeno/node/node/test/parallel/test-dgram-send-callback-recursive.js:15:11)
    at SendWrap.afterSend [as oncomplete] (dgram.js:383:8)

I've verified this test fails with node before the commit where this test was introduced: 18d457b and passes after this commit: b5cd2f0 when the libuv implementation of udp send was made asynchronous.

@jasnell jasnell added lts-watch-v4.x test Issues and PRs related to the tests. dgram Issues and PRs related to the dgram subsystem / UDP. labels Feb 4, 2016
@r-52
Copy link
Contributor

r-52 commented Feb 4, 2016

client.send(
chunk, 0, chunk.length, common.PORT, common.localhostIPv4, onsend);
} else {
assert.equal(async, true, 'Send should be asynchronous.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use strictEqual() here.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 4, 2016

LGTM with one comment.

Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.
@santigimeno santigimeno force-pushed the test-dgram-send-callback-recursive branch from f6c274b to 9616b88 Compare February 4, 2016 19:22
@santigimeno
Copy link
Member Author

Updated. Thanks!

@mcollina
Copy link
Member

LGTM

mcollina pushed a commit to mcollina/node that referenced this pull request Feb 26, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#5079
@mcollina
Copy link
Member

Landed in 8872840.

@mcollina mcollina closed this Feb 26, 2016
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
@Fishrock123 Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants