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

Investigate flaky test-net-error-twice #4057

Closed
Trott opened this issue Nov 28, 2015 · 4 comments
Closed

Investigate flaky test-net-error-twice #4057

Trott opened this issue Nov 28, 2015 · 4 comments
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform.

Comments

@Trott
Copy link
Member

Trott commented Nov 28, 2015

Example failure:

@Trott Trott added test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform. net Issues and PRs related to the net subsystem. and removed test Issues and PRs related to the tests. labels Nov 28, 2015
@Trott
Copy link
Member Author

Trott commented Nov 28, 2015

@Trott
Copy link
Member Author

Trott commented Nov 28, 2015

Another one:

Trott added a commit to Trott/io.js that referenced this issue Nov 28, 2015
The test was not reliably creating the error event on Windows 2012.

This makes the test more robust by delaying the start of the server
write until nextTick, allowing the connection to close and cause the
error more reliably on Windows 2012.

Fixes: nodejs#4057
@Trott
Copy link
Member Author

Trott commented Nov 29, 2015

PR to hopefully fix this: #4062

Trott added a commit to Trott/io.js that referenced this issue Nov 29, 2015
The test was not reliably creating the error event on Windows 2012.

This makes the test more robust by using setImmediate() to delay the
server writing to the socket to give time for socket.destroy() to take
effect.

Fixes: nodejs#4057
@joaocgreis
Copy link
Member

This failure reproduces every time when using a fast machine with a single logical processor, only in Windows 2012 R2.

This test was introduced in v0.10.26 and did not fail like this then. The problem started when v0.10 merged with v0.11, introducing this test there, meaning that v0.11 had the cause of this problem before. Using the test on previous revisions, the first bad commit is 2b6e078 . This does not help much, but is coherent with what @Trott observed in #4062 (comment) . Still investigating.

mscdex added a commit to mscdex/io.js that referenced this issue Dec 18, 2015
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: nodejs#4057
@mscdex mscdex closed this as completed in 4b0b991 Dec 19, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Dec 22, 2015
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: nodejs#4057
PR-URL: nodejs#4342
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: João Reis <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Jan 6, 2016
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: nodejs#4057
PR-URL: nodejs#4342
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: João Reis <[email protected]>
mscdex added a commit that referenced this issue Jan 7, 2016
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: #4057
PR-URL: #4342
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: João Reis <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 19, 2016
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: #4057
PR-URL: #4342
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: João Reis <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.

This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.

Fixes: nodejs#4057
PR-URL: nodejs#4342
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: João Reis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants