Closed
Bug 1295453
Opened 5 years ago
Closed 5 years ago
Intermittent dom/network/tests/test_tcpsocket_client_and_server_basics.html, dom/network/tests/test_tcpsocket_jsm.html | Test timed out.
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: wlach)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file)
Filed by: philringnalda [at] gmail.com https://treeherder.mozilla.org/logviewer.html#?job_id=33886059&repo=mozilla-inbound https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-macosx64/1471291096/mozilla-inbound_yosemite_r7_test-mochitest-other-bm135-tests1-macosx-build301.txt.gz
Comment hidden (Intermittent Failures Robot) |
Comment 2•5 years ago
|
||
This is pretty frequent and tends to cause failures in subsequent tests as well. Any chance you could take a look, Josh?
Flags: needinfo?(josh)
Comment 3•5 years ago
|
||
Looks like the same problem as bug 1295483.
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 9•5 years ago
|
||
FWIW, I'm looking into this. In the examples I've looked at, it seems like the problem is that a socket is being closed before the event we're expecting happens, and then we eventually just time out waiting for it to happen (which it never does). Not really sure why this would be the case, it's obviously *not supposed to happen* and nothing we're explicitly doing should trigger a close. Next steps: 1. See if we can reproduce this on a Linux machine 2. See if we can reproduce this under rr. It would indeed not surprise me if this was related to bug 1295483 or possibly other tcp oranges as well, but this seems like one of the simpler manifestations of what (might) be a larger problem, so let's start here.
Assignee | ||
Comment 10•5 years ago
|
||
Ok, I managed to reproduce this in rr (using chaos mode), though in retrospect I probably didn't need to: the bug is pretty simple, in some cases we close immediately before writing *any* data to the socket, so we wait for that event forever. There's no guarantee even 1 byte will be written, so we shouldn't expect it to. It should be sufficient to just wait for the connection to close, then to verify that a full set of data wasn't written (in theory this could still intermittently fail depending on timing, but the chances of that are infinitesimally small)
Assignee: nobody → wlachance
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•5 years ago
|
||
Note that the above patch basically reverts a change that :jdm requested in https://bugzilla.mozilla.org/show_bug.cgi?id=1104156#c28 ("We should first use serverQueue.waitForDataWithAtLeastLength - we want to verify that at least 1 byte is received before the close message, but we should also verify that the received data is less than the length of the two big arrays. Does that make sense?"), but I'm pretty sure that isn't the right thing to assert on. Would we expect that behaviour most of the time? Yes. Is it an error if we don't see it? No. (unless I'm missing something)
Assignee | ||
Updated•5 years ago
|
Summary: Intermittent dom/network/tests/test_tcpsocket_client_and_server_basics.html | Test timed out. → Intermittent dom/network/tests/test_tcpsocket_client_and_server_basics.html, dom/network/tests/test_tcpsocket_jsm.html | Test timed out.
Comment hidden (Intermittent Failures Robot) |
Comment 15•5 years ago
|
||
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 20•5 years ago
|
||
mozreview-review |
Comment on attachment 8786850 [details] Bug 1295453 - Don't wait for data to be sent when closing socket immediately https://reviewboard.mozilla.org/r/75734/#review74948 ::: dom/network/tests/test_tcpsocket_client_and_server_basics.js:382 (Diff revision 1) > - is(serverReceived.length < (2 * bigUint8Array.length), true, 'Received array length less than sent array length'); > - > is((yield serverQueue.waitForEvent()).type, 'close', > 'The close event is received after calling closeImmediately'); > > + is(serverReceived.length < (2 * bigUint8Array.length), true, 'Received array length less than sent array length'); This serverReceived value is unrelated to the sends that happened in this particular test, so we can't assert anything about this case now. What if we make a waitForAnyDataOrClose helper which uses Promise.race to catch both cases, and treat the close case as a length of 0?
Updated•5 years ago
|
Attachment #8786850 -
Flags: review?(josh) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•5 years ago
|
||
mozreview-review-reply |
Comment on attachment 8786850 [details] Bug 1295453 - Don't wait for data to be sent when closing socket immediately https://reviewboard.mozilla.org/r/75734/#review74948 > This serverReceived value is unrelated to the sends that happened in this particular test, so we can't assert anything about this case now. > > What if we make a waitForAnyDataOrClose helper which uses Promise.race to catch both cases, and treat the close case as a length of 0? Ah right, however I don't think we want Promise.race here, since we really do just want the close message to be called, at least as far as I understand things. I think this updated patch should work, let me know if I'm missing something.
Comment hidden (mozreview-request) |
Comment 24•5 years ago
|
||
mozreview-review |
Comment on attachment 8786850 [details] Bug 1295453 - Don't wait for data to be sent when closing socket immediately https://reviewboard.mozilla.org/r/75734/#review75264 Thanks!
Attachment #8786850 -
Flags: review?(josh) → review+
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 26•5 years ago
|
||
try run looks happy, I'm going to try to land: https://treeherder.mozilla.org/#/jobs?repo=try&revision=417cc6083b7e (missing data for runs on other platforms which I hoped to have probably because my try syntax is wrong, but I'm still pretty confident this won't break stuff)
Comment 27•5 years ago
|
||
Pushed by wlachance@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3d0b41fdd93b Don't wait for data to be sent when closing socket immediately r=jdm
Comment 28•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3d0b41fdd93b
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment hidden (Intermittent Failures Robot) |
Updated•2 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•