Bug 172943 – ASSERTION FAILED: RunLoop::isMain() in com.apple.WebKit: IPC::Connection::sendSyncMessage + 128

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

ASSERTION FAILED: RunLoop::isMain() in com.apple.WebKit: IPC::Connection::sendSyncMessage + 128:
Thread 2 Crashed:: Dispatch queue: com.apple.WebKit.StorageManager
0   com.apple.JavaScriptCore      	0x000000010855e3e4 WTFCrash + 36 (Assertions.cpp:323)
1   com.apple.WebKit              	0x000000010c7e7860 IPC::Connection::sendSyncMessage(unsigned long long, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >, WTF::Seconds, WTF::OptionSet<IPC::SendSyncOption>) + 128 (Connection.cpp:506)
2   com.apple.WebKit              	0x000000010c7e7283 IPC::Connection::sendMessage(std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >, WTF::OptionSet<IPC::SendOption>) + 1715 (Connection.cpp:387)
3   com.apple.WebKit              	0x000000010cca7432 bool IPC::Connection::send<Messages::StorageAreaMap::DidSetItem>(Messages::StorageAreaMap::DidSetItem&&, unsigned long long, WTF::OptionSet<IPC::SendOption>) + 994 (Connection.h:356)
4   com.apple.WebKit              	0x000000010cca7039 WebKit::StorageManager::setItem(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&) + 233 (StorageManager.cpp:792)
5   com.apple.WebKit              	0x000000010cccd2e8 void IPC::callMemberFunctionImpl<WebKit::StorageManager, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&), std::__1::tuple<unsigned long long, unsigned long long, unsigned long long, WTF::String, WTF::String, WTF::String>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul>(WebKit::StorageManager*, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&), IPC::Connection&, std::__1::tuple<unsigned long long, unsigned long long, unsigned long long, WTF::String, WTF::String, WTF::String>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul>) + 456 (HandleMessage.h:83)
6   com.apple.WebKit              	0x000000010ccccdf0 void IPC::callMemberFunction<WebKit::StorageManager, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&), std::__1::tuple<unsigned long long, unsigned long long, unsigned long long, WTF::String, WTF::String, WTF::String>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul> >(IPC::Connection&, std::__1::tuple<unsigned long long, unsigned long long, unsigned long long, WTF::String, WTF::String, WTF::String>&&, WebKit::StorageManager*, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&)) + 96 (HandleMessage.h:89)
7   com.apple.WebKit              	0x000000010cccbd4d void IPC::handleMessage<Messages::StorageManager::SetItem, WebKit::StorageManager, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&)>(IPC::Connection&, IPC::Decoder&, WebKit::StorageManager*, void (WebKit::StorageManager::*)(IPC::Connection&, unsigned long long, unsigned long long, unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&)) + 413 (HandleMessage.h:166)
8   com.apple.WebKit              	0x000000010cccb410 WebKit::StorageManager::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 656 (StorageManagerMessageReceiver.cpp:60)
9   com.apple.WebKit              	0x000000010c7e5686 IPC::Connection::dispatchWorkQueueMessageReceiverMessage(IPC::Connection::WorkQueueMessageReceiver&, IPC::Decoder&) + 102 (Connection.cpp:300)
10  com.apple.WebKit              	0x000000010c802b39 IPC::Connection::processIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_10::operator()() + 89 (Connection.cpp:672)
11  com.apple.WebKit              	0x000000010c8028e9 WTF::Function<void ()>::CallableWrapper<IPC::Connection::processIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_10>::call() + 25 (Function.h:89)
12  com.apple.JavaScriptCore      	0x000000010858d36e WTF::Function<void ()>::operator()() const + 94 (Function.h:50)
13  com.apple.JavaScriptCore      	0x00000001085de289 WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0::operator()() const + 25 (WorkQueueCocoa.cpp:37)
14  com.apple.JavaScriptCore      	0x00000001085de260 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0>(WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0)::'lambda'(void*)::operator()(void*) const + 32 (BlockPtr.h:86)
15  com.apple.JavaScriptCore      	0x00000001085de238 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0>(WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0)::'lambda'(void*)::__invoke(void*) + 24 (BlockPtr.h:85)
16  libdispatch.dylib             	0x00007fffc0728ef7 _dispatch_call_block_and_release + 12
17  libdispatch.dylib             	0x00007fffc07200b8 _dispatch_client_callout + 8
18  libdispatch.dylib             	0x00007fffc0736ae5 _dispatch_queue_serial_drain + 896
19  libdispatch.dylib             	0x00007fffc0728cd9 _dispatch_queue_invoke + 1046
20  libdispatch.dylib             	0x00007fffc0721e70 _dispatch_root_queue_drain + 476
21  libdispatch.dylib             	0x00007fffc0721c47 _dispatch_worker_thread3 + 99
22  libsystem_pthread.dylib       	0x00007fffc096d712 _pthread_wqthread + 1299
23  libsystem_pthread.dylib       	0x00007fffc096d1ed start_wqthread + 13

[Chris Dumez]

<rdar://problem/31288058>

[Chris Dumez]

Created attachment 312038 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 312038 [details]
Patch

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

Very nice!

> LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:7
> +description("Tests updatinh sessionStorage in the keydown handler and makes sure the value is updated synchronously.");

Typo: updatinh.

[Chris Dumez]

Created attachment 312041 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 312041 [details]
Patch

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

> LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:12
> +    sessionStorage.testValue = 1;

Oh, won't this persist to other tests? Should have a unique name like setItemSynchronousKeydownTestValue (removing it at the end would be useful too).

[Chris Dumez]

Comment on attachment 312041 [details]
Patch

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

>> LayoutTests/storage/domstorage/sessionstorage/set-item-synchronous-keydown.html:12
>> +    sessionStorage.testValue = 1;
> 
> Oh, won't this persist to other tests? Should have a unique name like setItemSynchronousKeydownTestValue (removing it at the end would be useful too).

Hmm, it looks like other sessionStorage tests include <script src="resources/clearSessionStorage.js"></script> so they clear everything before running the test. I guess I should do the same.

[Chris Dumez]

Created attachment 312042 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 312042 [details]
Patch

Clearing flags on attachment: 312042

Committed r217810: <http://trac.webkit.org/changeset/217810>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

+sessionStorage.removeItem("setItemSynchronousKeydownTestValue");

This should at least be in its own <script></script> block, to defend against exceptions in the main test body. Currently, run-webkit-tests restarts the UI process for crashes and freezes anyway, but in the general case, a crash or freeze could make the value leak.

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #10)
> +sessionStorage.removeItem("setItemSynchronousKeydownTestValue");
> 
> This should at least be in its own <script></script> block, to defend
> against exceptions in the main test body. Currently, run-webkit-tests
> restarts the UI process for crashes and freezes anyway, but in the general
> case, a crash or freeze could make the value leak.

Ok. Although other tests do not have any kind of completion clean up it seems.

[Chris Dumez]

Reopen for follow-up.

[Chris Dumez]

Created attachment 312043 [details]
Follow-up fix

[WebKit Commit Bot]

Comment on attachment 312043 [details]
Follow-up fix

Clearing flags on attachment: 312043

Committed r217813: <http://trac.webkit.org/changeset/217813>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #8)
> Comment on attachment 312042 [details]
> Patch
> 
> Clearing flags on attachment: 312042
> 
> Committed r217810: <http://trac.webkit.org/changeset/217810>
I skipped the test for this change on iOS with http://trac.webkit.org/projects/webkit/changeset/217820 since it relies upon eventSender.keyDown()