Bug 141189 – Web Inspector: ASSERT mainThreadPthread launching remote debuggable JSContext app with Debug JavaScriptCore

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

[Joseph Pecoraro]

* SUMMARY
ASSERT mainThreadPthread launching remote debuggable JSContext app with Debug JavaScriptCore.

    ASSERTION FAILED: mainThreadPthread
    /Users/pecoraro/Code/safari/OpenSource/Source/WTF/wtf/mac/MainThreadMac.mm(206) : bool WTF::isMainThread()
    1   0x100819d40 WTFCrash
    2   0x10084374c WTF::isMainThread()
    3   0x100866271 WTF::StringImpl::createCFString()
    4   0x1008667f4 WTF::StringImpl::operator NSString*()
    5   0x100759cc7 WTF::String::operator NSString*() const
    6   0x100758a8a Inspector::RemoteInspector::listingForDebuggable(Inspector::RemoteInspectorDebuggableInfo const&) const
    7   0x100758f9c Inspector::RemoteInspector::pushListingNow()
    8   0x100758289 Inspector::RemoteInspector::receivedGetListingMessage(NSDictionary*)
    9   0x100757820 Inspector::RemoteInspector::xpcConnectionReceivedMessage(Inspector::RemoteInspectorXPCConnection*, NSString*, NSDictionary*)
    10  0x100765dc8 Inspector::RemoteInspectorXPCConnection::handleEvent(void*)
    ...

This is because WTF::initializeMainThread() hasn't happened yet, which initializes mainThreadPthread. It must be called from the main thread.

Lets just do it before we auto-start the RemoteInspector::singleton.

[Joseph Pecoraro]

Created attachment 245924 [details]
[PATCH] Proposed Fix

I have tested on OS X (JSContext, Safari). I will need to test iOS (JSContext, UIWebView) before landing, so marking as cq- for now.

[Joseph Pecoraro]

<rdar://problem/17721044>

[Michael Saboff]

Comment on attachment 245924 [details]
[PATCH] Proposed Fix

r=me

[Geoffrey Garen]

Comment on attachment 245924 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/remote/RemoteInspector.mm:91
> +                    shared.get().start();

I believe that start() used to be called on the XPC queue, and now will be called on the main queue.

I think it would be slightly better to dispatch *again* inside the main queue block *back* to the XPC queue in order to call start().

[Joseph Pecoraro]

Comment on attachment 245924 [details]
[PATCH] Proposed Fix

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

>> Source/JavaScriptCore/inspector/remote/RemoteInspector.mm:91
>> +                    shared.get().start();
> 
> I believe that start() used to be called on the XPC queue, and now will be called on the main queue.
> 
> I think it would be slightly better to dispatch *again* inside the main queue block *back* to the XPC queue in order to call start().

start() was getting called on whatever queue/thread first called RemoteInspector::singleton. Often, that is whatever queue/thread created the first JSContext/WebCore::Page/

It is thread safe and can be called from any queue:

  void RemoteInspector::start()
  {
      std::lock_guard<std::mutex> lock(m_mutex);
      ...
  }

Is your concern that we should do work off the main queue?

[Geoffrey Garen]

> start() was getting called on whatever queue/thread first called
> RemoteInspector::singleton. Often, that is whatever queue/thread created the
> first JSContext/WebCore::Page/

Okeedokee.

> Is your concern that we should do work off the main queue?

I'm not necessarily concerned that we must work off the main queue; I was just observing (possibly incorrectly) that the previous code seemed to avoid the startup on the main queue.

[Joseph Pecoraro]

(In reply to comment #6)
> > start() was getting called on whatever queue/thread first called
> > RemoteInspector::singleton. Often, that is whatever queue/thread created the
> > first JSContext/WebCore::Page/
> 
> Okeedokee.
> 
> > Is your concern that we should do work off the main queue?
> 
> I'm not necessarily concerned that we must work off the main queue; I was
> just observing (possibly incorrectly) that the previous code seemed to avoid
> the startup on the main queue.

Ahh, okay. It was doing the work on whatever queue it was given. Originally I had it do WTF::initializeMainThread(), which was incorrect if it was initialized on a non-main queue. Now I'm just biting the bullet and always doing it on the main queue.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/179562