Bug 152213 – Web Inspector: Too many derefs when RemoteInspectorXPCConnection fails to validate connection

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Too many derefs when RemoteInspectorXPCConnection fails to validate connection.

We call closeOnQueue() and deref(), but the closeOnQueue will cause an XPC_ERROR_CONNECTION_INVALID to deref() the connection again. We should only deref in one place, this code should just trigger the close.

[Radar WebKit Bug Importer]

<rdar://problem/23870973>

[Joseph Pecoraro]

Created attachment 267223 [details]
[PATCH] Proposed Fix

[Ryosuke Niwa]

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?

[Joseph Pecoraro]

(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.

[Joseph Pecoraro]

<http://trac.webkit.org/changeset/194005>