141189 2015-02-02 21:13:38 -0800 Web Inspector: ASSERT mainThreadPthread launching remote debuggable JSContext app with Debug JavaScriptCore 2015-02-03 12:23:47 -0800 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED DoNotImportToRadar P2 Normal --- 1 joepeck joepeck ggaren graouts joepeck jonowells mattbaker msaboff nvasilyev timothy webkit-bug-importer oldest_to_newest 1066354 0 joepeck 2015-02-02 21:13:38 -0800 * 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. 1066355 1 245924 joepeck 2015-02-02 21:15:59 -0800 Created attachment 245924 [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. 1066356 2 joepeck 2015-02-02 21:16:25 -0800 <rdar://problem/17721044> 1066362 3 245924 msaboff 2015-02-02 21:35:56 -0800 Comment on attachment 245924 [PATCH] Proposed Fix r=me 1066500 4 245924 ggaren 2015-02-03 11:04:13 -0800 Comment on attachment 245924 [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(). 1066511 5 245924 joepeck 2015-02-03 11:34:06 -0800 Comment on attachment 245924 [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? 1066515 6 ggaren 2015-02-03 11:40:45 -0800 > 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. 1066518 7 joepeck 2015-02-03 11:46:18 -0800 (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. 1066524 8 joepeck 2015-02-03 12:23:47 -0800 http://trac.webkit.org/changeset/179562 245924 2015-02-02 21:15:59 -0800 2015-02-02 21:35:56 -0800 [PATCH] Proposed Fix initialize.patch text/plain 1714 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDA3Yzk4MzAuLmJmODlmMGMgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE1LTAyLTAyICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogQVNTRVJU IG1haW5UaHJlYWRQdGhyZWFkIGxhdW5jaGluZyByZW1vdGUgZGVidWdnYWJsZSBKU0NvbnRleHQg YXBwIHdpdGggRGVidWcgSmF2YVNjcmlwdENvcmUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTE0MTE4OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgICogaW5zcGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3Iu bW06CisgICAgICAgIChJbnNwZWN0b3I6OlJlbW90ZUluc3BlY3Rvcjo6c2luZ2xldG9uKToKKyAg ICAgICAgRW5zdXJlIHdlIGNhbGwgV1RGOjppbml0aWFsaXplTWFpblRocmVhZCgpIG9uIHRoZSBt YWluIHRocmVhZCBzbyB0aGF0CisgICAgICAgIHdlIGNhbiBwZXJmb3JtIGF1dG9tYXRpYyBTdHJp bmcgPC0+IE5TU3RyaW5nIGNvbnZlcnNpb25zLgorCiAyMDE1LTAyLTAyICBHZW9mZnJleSBHYXJl biAgPGdnYXJlbkBhcHBsZS5jb20+CiAKICAgICAgICAgVXNlIEZhc3RNYWxsb2MgKGJtYWxsb2Mp IGluc3RlYWQgb2YgQmxvY2tBbGxvY2F0b3IgZm9yIEdDIHBhZ2VzCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvaW5zcGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3IubW0gYi9T b3VyY2UvSmF2YVNjcmlwdENvcmUvaW5zcGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3IubW0K aW5kZXggZWE0YTg2Zi4uYTM2ZDMyZCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3Jl L2luc3BlY3Rvci9yZW1vdGUvUmVtb3RlSW5zcGVjdG9yLm1tCisrKyBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS9pbnNwZWN0b3IvcmVtb3RlL1JlbW90ZUluc3BlY3Rvci5tbQpAQCAtODQsOSArODQs MTIgQEAgUmVtb3RlSW5zcGVjdG9yJiBSZW1vdGVJbnNwZWN0b3I6OnNpbmdsZXRvbigpCiAgICAg c3RhdGljIGRpc3BhdGNoX29uY2VfdCBvbmNlOwogICAgIGRpc3BhdGNoX29uY2UoJm9uY2UsIF57 CiAgICAgICAgIGlmIChjYW5BY2Nlc3NXZWJJbnNwZWN0b3JNYWNoUG9ydCgpKSB7Ci0gICAgICAg ICAgICBKU0M6OmluaXRpYWxpemVUaHJlYWRpbmcoKTsKLSAgICAgICAgICAgIGlmIChSZW1vdGVJ bnNwZWN0b3I6OnN0YXJ0RW5hYmxlZCkKLSAgICAgICAgICAgICAgICBzaGFyZWQuZ2V0KCkuc3Rh cnQoKTsKKyAgICAgICAgICAgIGRpc3BhdGNoX2FzeW5jKGRpc3BhdGNoX2dldF9tYWluX3F1ZXVl KCksIF57CisgICAgICAgICAgICAgICAgV1RGOjppbml0aWFsaXplTWFpblRocmVhZCgpOworICAg ICAgICAgICAgICAgIEpTQzo6aW5pdGlhbGl6ZVRocmVhZGluZygpOworICAgICAgICAgICAgICAg IGlmIChSZW1vdGVJbnNwZWN0b3I6OnN0YXJ0RW5hYmxlZCkKKyAgICAgICAgICAgICAgICAgICAg c2hhcmVkLmdldCgpLnN0YXJ0KCk7CisgICAgICAgICAgICB9KTsKICAgICAgICAgfQogICAgIH0p OwogCg==