Bug 172585 - [Curl] Gracefully handle failures in websockets
Summary: [Curl] Gracefully handle failures in websockets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-25 01:08 PDT by isaac+webkit
Modified: 2018-07-24 19:11 PDT (History)
6 users (show)

See Also:


Attachments
Patch (9.65 KB, patch)
2017-05-25 02:07 PDT, isaac+webkit
achristensen: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (922.78 KB, application/zip)
2017-05-25 03:31 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description isaac+webkit 2017-05-25 01:08:32 PDT
Gracefully handle failures in websockets on wincairo
Comment 1 isaac+webkit 2017-05-25 02:07:28 PDT
Created attachment 311211 [details]
Patch
Comment 2 Build Bot 2017-05-25 03:31:50 PDT
Comment on attachment 311211 [details]
Patch

Attachment 311211 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3811988

New failing tests:
fetch/closing-while-fetching-blob.html
Comment 3 Build Bot 2017-05-25 03:31:51 PDT
Created attachment 311213 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 4 Alex Christensen 2017-07-05 14:06:29 PDT
Comment on attachment 311211 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311211&action=review

> Source/WebCore/ChangeLog:3
> +        Handle failures in websocket on wincairo

This is a great thing.  Sorry I didn't notice this patch earlier.  Let's polish it up and land it.

> Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp:283
> +            callOnMainThread([this, ret] {
> +                // Check reference count to fix a crash.

This is a bad pattern I think we shouldn't expand upon.  Capture protectedThis = makeRef(*this) to make sure this isn't deallocated underneath us while waiting for the lambda to be called on another thread.
Comment 5 Don Olmstead 2018-01-08 12:32:27 PST
Issac is this something you're going to complete? If not could you let us know so we can assign someone over to take a look?
Comment 6 isaac+webkit 2018-01-08 12:48:36 PST
Hi Don, Sorry I'm not working on this anymore, feel free to take this over.
Comment 7 Basuke Suzuki 2018-07-24 19:11:39 PDT
This is fixed in https://bugs.webkit.org/show_bug.cgi?id=172630.