89153 2012-06-14 18:48:54 -0700 http/tests/websocket/tests/hybi/workers/close.html is flaky 2026-01-27 08:58:40 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All ASSIGNED https://bugs.webkit.org/show_bug.cgi?id=171830 P2 Normal --- 1 yutak li.yin aboya ahmad.saleem792 ap bank cdumez jlewis3 Lawrence.j li.yin ossy tkent toyoshim oldest_to_newest 649734 0 yutak 2012-06-14 18:48:54 -0700 Chromium bug: http://code.google.com/p/chromium/issues/detail?id=132898 Layout test http/tests/websocket/tests/hybi/workers/close.html is failing flakily on Chromium bots on all platforms: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=http%2Ftests%2Fwebsocket%2Ftests%2Fhybi%2Fworkers%2Fclose.html Li, apparently this flakiness started from your patch at r120291. Could you take a look? 649770 1 li.yin 2012-06-14 19:26:16 -0700 Okay, I will investigate it ASAP. Sorry for that. 649815 2 li.yin 2012-06-14 20:49:51 -0700 I can't reproduce this issue. I check the newest code from trunk, and http/tests/websocket/tests/hybi/workers/close.html works on chromium-gtk and WebKit-gtk+ port. 649871 3 yutak 2012-06-14 22:10:56 -0700 * You might have a better luck with Debug builds. * You might want to repeat to run the test as this failure is flaky. At least I can reproduce the failure pretty reliably on Chromium ToT with Linux+Debug. I can observe the similar flakiness in a browser as well as DumpRenderTree. Diff always looks like the following: --- /b/build/slave/Webkit_Linux__dbg_/build/layout-test-results/http/tests/websocket/tests/hybi/workers/close-expected.txt +++ /b/build/slave/Webkit_Linux__dbg_/build/layout-test-results/http/tests/websocket/tests/hybi/workers/close-actual.txt @@ -45,7 +45,6 @@ Invalid code test: 12 Code NaN must cause INVALID_ACCESS_ERR. PASS PASS: worker: exceptionName is invalidAccessErr -PASS PASS: onerror() was called. runCodeTest: onclose(). PASS PASS: worker: closeEvent.code is abnormalClosure Skip invalid string test. 649891 4 li.yin 2012-06-14 22:43:32 -0700 (In reply to comment #3) > * You might have a better luck with Debug builds. > * You might want to repeat to run the test as this failure is flaky. > > At least I can reproduce the failure pretty reliably on Chromium ToT with Linux+Debug. I can observe the similar flakiness in a browser as well as DumpRenderTree. > > Diff always looks like the following: > > --- /b/build/slave/Webkit_Linux__dbg_/build/layout-test-results/http/tests/websocket/tests/hybi/workers/close-expected.txt > +++ /b/build/slave/Webkit_Linux__dbg_/build/layout-test-results/http/tests/websocket/tests/hybi/workers/close-actual.txt > @@ -45,7 +45,6 @@ > Invalid code test: 12 > Code NaN must cause INVALID_ACCESS_ERR. > PASS PASS: worker: exceptionName is invalidAccessErr > -PASS PASS: onerror() was called. > runCodeTest: onclose(). > PASS PASS: worker: closeEvent.code is abnormalClosure > Skip invalid string test. Oh, I tested it in Release mode of chromium-gtk and WebKit-gtk+ port. According to your words and my test, that is to say, the test is failed just in debug mode. 649922 5 li.yin 2012-06-14 23:32:23 -0700 I reproduce it on Chromium-gtk Debug mode. I will follow up. Thanks for your reporting. 654447 6 148783 li.yin 2012-06-21 06:31:55 -0700 Created attachment 148783 Patch 654468 7 li.yin 2012-06-21 06:53:18 -0700 This is a timing related issue. Between ThreadableWebSocketChannelClientWrapper::didReceiveMessageError and ThreadableWebSocketChannelClientWrapper::didReceiveMessageErrorCallback, suppose WorkerThreadableWebSocketChannel::Peer::didConnect() is invoked, then WebSocket::didConnect will be invoked, beacuse the m_state is changed to be CLOSING by WebSocket::close (http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/websockets/WebSocket.cpp#L373). It will call didClose directly, WorkerThreadableWebSocketChannel::disconnect will be invoked. It will clearClientWrapper. So wrapper->m_client->didReceiveMessageError() can't be invoked beacuse wrapper->m_client is clear. (http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp#L293) 660266 8 ossy 2012-06-29 01:20:48 -0700 It is valid bug on Qt too. 661231 9 li.yin 2012-07-01 19:20:42 -0700 (In reply to comment #8) > It is valid bug on Qt too. Is the result same with chromium? If so, this patch can fix the issue. 666216 10 li.yin 2012-07-10 17:53:11 -0700 Hi Yuta, Would you please review this patch? Thanks in advance. 668702 11 148783 yutak 2012-07-13 00:57:11 -0700 Comment on attachment 148783 Patch I'm not yet sure that this fix is the right fix; my current feeling is that this patch just hides the problem and doesn't fix the root cause. Could you tell me the detailed scenario that causes didClose() callback to be called earlier than didReceiveMessageError() callback? There should be some race inside communication between main thread and worker thread, and my gut feeling is that WebSocket.cpp doesn't need to be changed to fix this issue (but I might be wrong). 669971 12 li.yin 2012-07-15 06:36:46 -0700 (In reply to comment #11) > (From update of attachment 148783 [details]) > I'm not yet sure that this fix is the right fix; my current feeling is that this patch just hides the problem and doesn't fix the root cause. > > Could you tell me the detailed scenario that causes didClose() callback to be called earlier than didReceiveMessageError() callback? There should be some race inside communication between main thread and worker thread, and my gut feeling is that WebSocket.cpp doesn't need to be changed to fix this issue (but I might be wrong). When the WebSocket state is connecting, calling close will invoke the didReceiveMessageError function, If the context is workerContext, the didReceiveMessageErrorCallback will be added into event loop, can't be executed immediately. Suppose in that time, the socket is connected, it will trigger WebSocket::didConnect. Here (http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/websockets/WebSocket.cpp#L451), it will cause error. In WorkerContext, the task should be added into event loop, so it can work in Doeument, not WorkerContext. 677842 13 bank 2012-07-25 02:45:27 -0700 Added to Qt skiplist in r123595. 678859 14 li.yin 2012-07-25 22:31:30 -0700 The root cause is that we should add the task into EventLoop queue in WorkerContext, but WebSocket::didConnect (http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/websockets/WebSocket.cpp#L451) can't follow it. Hi Yuta, I wander if my clarification is more clearer? Any question, please feel free to let me know. 976858 15 148783 andersca 2014-02-05 10:57:25 -0800 Comment on attachment 148783 Patch Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag. 1001022 16 ap 2014-04-15 16:20:22 -0700 *** Bug 131716 has been marked as a duplicate of this bug. *** 1001023 17 ap 2014-04-15 16:20:46 -0700 Marked as flaky in <http://trac.webkit.org/r167332> 2024905 18 ahmad.saleem792 2024-03-29 22:16:15 -0700 https://chromium.googlesource.com/chromium/blink/+/977662eb6360bab2875aafddfedd2b753aaf9fc7 We have 'http/tests/websocket/tests/hybi/workers/close.html [ Pass Failure ]' expectations for 'iOS' and 'mac'. 2175447 19 aboya 2026-01-27 08:57:54 -0800 This test fails as well in WPE and GTK, so I'll make the expectations cover all platforms as there is little chance this is platform specific. 2175449 20 aboya 2026-01-27 08:58:18 -0800 *** Bug 207156 has been marked as a duplicate of this bug. *** 2175451 21 aboya 2026-01-27 08:58:40 -0800 *** Bug 171830 has been marked as a duplicate of this bug. *** 148783 2012-06-21 06:31:55 -0700 2014-02-05 10:57:25 -0800 Patch bug-89153-20120622213540.patch text/plain 3076 li.yin U3VidmVyc2lvbiBSZXZpc2lvbjogMTIwMzkzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTc4MGRkZDlmMmJkM2Zi MTkwNzg4MmUzZDBlMzRjMzE2YjI5ZmMxNi4uYTlhYTdjZmViMWRlMDRhZWNlZjcyZDFjMWM4OTZh NGMzMTMxNGNlOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA2LTIyICBMaSBZ aW4gIDxsaS55aW5AaW50ZWwuY29tPgorCisgICAgICAgIGh0dHAvdGVzdHMvd2Vic29ja2V0L3Rl c3RzL2h5Ymkvd29ya2Vycy9jbG9zZS5odG1sIGlzIGZsYWt5CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTE1MworCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEluIGRpZENvbm5lY3QgY2FsbGJhY2ssIHRoZXJl IGlzIG5vdCBuZWVkIHRvIGNhbGwgZGlkQ2xvc2UsIGlmIHN0YXRlIGlzCisgICAgICAgIENMT1NJ TkcsIGRpZENsb3NlIHdpbGwgYmUgaW52b2tlZCBieSBpdHNlbGYsIGFuZCBpdCBpcyBwb3NzaWJs ZSB0byBjYXVzZQorICAgICAgICBzb21lIGRpZmZlcmVudCByZXN1bHRzIGJldHdlZW4gZG9jdW1l bnQgYW5kIHdvcmtlciBjb250ZXh0LgorCisgICAgICAgIFRlc3Q6IGh0dHAvdGVzdHMvd2Vic29j a2V0L3Rlc3RzL2h5Ymkvd29ya2Vycy9jbG9zZS5odG1sCisKKyAgICAgICAgKiBNb2R1bGVzL3dl YnNvY2tldHMvV2ViU29ja2V0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OldlYlNvY2tldDo6ZGlk Q29ubmVjdCk6CisKIDIwMTItMDYtMTQgIFRvbnkgUGF5bmUgIDx0cGF5bmVAY2hyb21pdW0ub3Jn PgogCiAgICAgICAgW2Nocm9taXVtXSBBZGQgaWNjanBlZyBhbmQgcWNtcyB0byBjaHJvbWl1bSBw b3J0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3dlYnNvY2tldHMvV2ViU29j a2V0LmNwcCBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvd2Vic29ja2V0cy9XZWJTb2NrZXQuY3Bw CmluZGV4IDNjNWM1OWM2YTI5MmJjZDhmYTBiZWY2ZDE2NGUxZDBhYWRhMDZjMjEuLjlkZThhNzc0 NGU2MzQ2OGQ4ZmUyMTI0YjQ0NWVhYTMwNjJlM2E5NzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL01vZHVsZXMvd2Vic29ja2V0cy9XZWJTb2NrZXQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3Jl L01vZHVsZXMvd2Vic29ja2V0cy9XZWJTb2NrZXQuY3BwCkBAIC00ODgsMTAgKzQ4OCw4IEBAIHZv aWQgV2ViU29ja2V0OjpzdG9wKCkKIHZvaWQgV2ViU29ja2V0OjpkaWRDb25uZWN0KCkKIHsKICAg ICBMT0coTmV0d29yaywgIldlYlNvY2tldCAlcCBkaWRDb25uZWN0IiwgdGhpcyk7Ci0gICAgaWYg KG1fc3RhdGUgIT0gQ09OTkVDVElORykgewotICAgICAgICBkaWRDbG9zZSgwLCBDbG9zaW5nSGFu ZHNoYWtlSW5jb21wbGV0ZSwgV2ViU29ja2V0Q2hhbm5lbDo6Q2xvc2VFdmVudENvZGVBYm5vcm1h bENsb3N1cmUsICIiKTsKKyAgICBpZiAobV9zdGF0ZSAhPSBDT05ORUNUSU5HKQogICAgICAgICBy ZXR1cm47Ci0gICAgfQogICAgIEFTU0VSVChzY3JpcHRFeGVjdXRpb25Db250ZXh0KCkpOwogICAg IG1fc3RhdGUgPSBPUEVOOwogICAgIG1fc3VicHJvdG9jb2wgPSBtX2NoYW5uZWwtPnN1YnByb3Rv Y29sKCk7CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9D aGFuZ2VMb2cKaW5kZXggMTM1Y2M0NmQ4ZmMzODVkNjMyYzIwN2ExNmZhYmJlNThlMDAxZjM2My4u OWIwZDIxYWMzMTBiMWU5MTkwZWVjYzEyNzQ4OTc4ODQzYjg1MDRkZSAxMDA2NDQKLS0tIGEvTGF5 b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE1IEBACisyMDEyLTA2LTIyICBMaSBZaW4gIDxsaS55aW5AaW50ZWwuY29tPgorCisgICAgICAg IGh0dHAvdGVzdHMvd2Vic29ja2V0L3Rlc3RzL2h5Ymkvd29ya2Vycy9jbG9zZS5odG1sIGlzIGZs YWt5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04OTE1 MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJlbW92 ZSBodHRwL3Rlc3RzL3dlYnNvY2tldC90ZXN0cy9oeWJpL3dvcmtlcnMvY2xvc2UuaHRtbCBmcm9t IGNocm9taXVtIHBsYXRmb3JtCisgICAgICAgIFRlc3RFeHBlY3RhdGlvbnMuCisKKyAgICAgICAg KiBwbGF0Zm9ybS9jaHJvbWl1bS9UZXN0RXhwZWN0YXRpb25zOgorCiAyMDEyLTA2LTE0ICBTaGVy aWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmll d2VkLCByb2xsaW5nIG91dCByMTIwMzg0LgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvcGxhdGZv cm0vY2hyb21pdW0vVGVzdEV4cGVjdGF0aW9ucyBiL0xheW91dFRlc3RzL3BsYXRmb3JtL2Nocm9t aXVtL1Rlc3RFeHBlY3RhdGlvbnMKaW5kZXggNTlhZGI4NWI4ZTg0YTk5YjA5Y2M3NWUzOTgzZTUz NTBjYjNlN2NjOS4uY2JmN2ZhNzQ3ZmIwNGM0MzUxYWZiZmJjODFlZGUxZDZkM2NlODgwMCAxMDA2 NDQKLS0tIGEvTGF5b3V0VGVzdHMvcGxhdGZvcm0vY2hyb21pdW0vVGVzdEV4cGVjdGF0aW9ucwor KysgYi9MYXlvdXRUZXN0cy9wbGF0Zm9ybS9jaHJvbWl1bS9UZXN0RXhwZWN0YXRpb25zCkBAIC0z ODE3LDYgKzM4MTcsNCBAQCBCVUdXSzg4ODMyIFhQIDogaHR0cC90ZXN0cy94bWxodHRwcmVxdWVz dC9vcmlnaW4tZXhhY3QtbWF0Y2hpbmcuaHRtbCA9IFBBU1MgVElNRQogCiBCVUdXSzg4ODU2IDog ZmFzdC9ldmVudHMvY29uc3RydWN0b3JzL3NwZWVjaC1yZWNvZ25pdGlvbi1ldmVudC1jb25zdHJ1 Y3Rvci5odG1sID0gVEVYVAogCi1CVUdDUjEzMjg5OCA6IGh0dHAvdGVzdHMvd2Vic29ja2V0L3Rl c3RzL2h5Ymkvd29ya2Vycy9jbG9zZS5odG1sID0gVEVYVAotCiBCVUdXSzg5MTQ5IDogaHR0cC90 ZXN0cy9maWxlYXBpL2NyZWF0ZS1ibG9iLXVybC1mcm9tLWRhdGEtdXJsLmh0bWwgPSBURVhUCg==