Bug 144057 - http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html crashes on Windows almost all the time
Summary: http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html crashes ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-22 11:33 PDT by Alexey Proskuryakov
Modified: 2016-05-30 13:56 PDT (History)
10 users (show)

See Also:


Attachments
Patch (2.16 KB, patch)
2016-05-30 02:18 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (872.42 KB, application/zip)
2016-05-30 02:54 PDT, Build Bot
no flags Details
Patch (1.95 KB, patch)
2016-05-30 07:12 PDT, Per Arne Vollan
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-04-22 11:33:25 PDT
http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html crashes on Windows almost every time:

https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fwebsocket%2Ftests%2Fhybi%2Fstop-on-resume-in-error-handler.html
Comment 1 Chris Dumez 2015-04-23 10:13:13 PDT
Sadly the Windows bot doesn't give us a trace. I am going to skip the test for now and look into this when I find time.
Comment 2 Chris Dumez 2015-04-23 10:18:12 PDT
Temporarily skipped in <http://trac.webkit.org/changeset/183191>.
Comment 3 Radar WebKit Bug Importer 2015-05-05 17:13:30 PDT
<rdar://problem/20829027>
Comment 4 Brent Fulgham 2016-03-22 10:21:08 PDT
This seems to only happen in Release builds.
Comment 5 Per Arne Vollan 2016-05-30 02:18:54 PDT
Created attachment 280086 [details]
Patch
Comment 6 Build Bot 2016-05-30 02:54:47 PDT
Comment on attachment 280086 [details]
Patch

Attachment 280086 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1406444

Number of test failures exceeded the failure limit.
Comment 7 Build Bot 2016-05-30 02:54:52 PDT
Created attachment 280087 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Per Arne Vollan 2016-05-30 07:12:48 PDT
Created attachment 280098 [details]
Patch
Comment 9 Brent Fulgham 2016-05-30 09:37:55 PDT
Comment on attachment 280098 [details]
Patch

Thank you for fixing this! Can you unskip the test when you land the fix?
Comment 10 Per Arne Vollan 2016-05-30 09:40:40 PDT
(In reply to comment #9)
> Comment on attachment 280098 [details]
> Patch
> 
> Thank you for fixing this! Can you unskip the test when you land the fix?

Will do :) Thanks for reviewing!
Comment 11 Per Arne Vollan 2016-05-30 12:05:10 PDT
Committed r201500: <http://trac.webkit.org/changeset/201500>
Comment 12 Per Arne Vollan 2016-05-30 12:06:37 PDT
Updated test expectations in <http://trac.webkit.org/changeset/201501>.
Comment 13 Chris Dumez 2016-05-30 12:12:17 PDT
Comment on attachment 280098 [details]
Patch

Not sure how the protector helps since it is not capture by value in the lambda?
Comment 14 Per Arne Vollan 2016-05-30 13:03:24 PDT
(In reply to comment #13)
> Comment on attachment 280098 [details]
> Patch
> 
> Not sure how the protector helps since it is not capture by value in the
> lambda?

I added this to make sure the SocketStreamHandle object is not deleted before the call is executed on the main thread, and used a RefPtr since the thread will wait for the call to finish. Perhaps this is not needed?
Comment 15 Chris Dumez 2016-05-30 13:39:50 PDT
Oh, I thought this was a callOnMainThread() call. I am not familiar with callOnMainThreadAndWait() but your change is probably fine then.
Comment 16 Per Arne Vollan 2016-05-30 13:56:01 PDT
(In reply to comment #15)
> Oh, I thought this was a callOnMainThread() call. I am not familiar with
> callOnMainThreadAndWait() but your change is probably fine then.

Sounds good, I actually think this file is the only place where callOnMainThreadAndWait() is used :)