Summary: | Web Inspector: Too many derefs when RemoteInspectorXPCConnection fails to validate connection | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bburg, commit-queue, graouts, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, rniwa, saam, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Other | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2015-12-11 23:27:43 PST
Created attachment 267223 [details]
[PATCH] Proposed Fix
Comment on attachment 267223 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=267223&action=review rs=me > Source/JavaScriptCore/inspector/remote/RemoteInspectorXPCConnection.mm:-182 > - m_closed = true; > - m_client = nullptr; It looks like we always evaluate these two statements before calling closeOnQueue() elsewhere. Is it really safe not to set these values? (In reply to comment #3) > Comment on attachment 267223 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267223&action=review > > rs=me > > > Source/JavaScriptCore/inspector/remote/RemoteInspectorXPCConnection.mm:-182 > > - m_closed = true; > > - m_client = nullptr; > > It looks like we always evaluate these two statements before calling > closeOnQueue() elsewhere. > Is it really safe not to set these values? Thanks for the careful review! I don't think it is necessary here. We don't want to set m_client = nullptr yet, because we do want to inform the client that we are closing, which will happen in the handling of XPC_ERROR_CONNECTION_INVALID. The case of `close` and `closeFromMessage` it is the client doing the closing, so they don't need to be informed that the connection is closing. We could set m_close, but we might as well delay that until when we actually do the closing with XPC_ERROR_CONNECTION_INVALID triggered by this. So waiting and closing there feels more natural. |