Crash under SWServer::unregisterConnection(Connection&): Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010a63bc60 WTFCrash + 16 (Assertions.cpp:267) 1 com.apple.WebCore 0x000000011562690a WebCore::SWServer::unregisterConnection(WebCore::SWServer::Connection&) + 154 (SWServer.cpp:704) 2 com.apple.WebCore 0x0000000115626842 WebCore::SWServer::Connection::~Connection() + 50 (SWServer.cpp:63) 3 com.apple.WebKit 0x00000001046e809b WebKit::WebSWServerConnection::~WebSWServerConnection() + 411 (WebSWServerConnection.cpp:77) 4 com.apple.WebKit 0x00000001046e82d5 WebKit::WebSWServerConnection::~WebSWServerConnection() + 21 (WebSWServerConnection.cpp:77) 5 com.apple.WebKit 0x00000001046e8339 WebKit::WebSWServerConnection::~WebSWServerConnection() + 25 (WebSWServerConnection.cpp:73) 6 com.apple.WebKit 0x0000000103ee57f5 WTF::VectorDestructor<true, std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> > >::destruct(std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >*, std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >*) + 213 (memory:2598) 7 com.apple.WebKit 0x0000000103ee56ad WTF::VectorTypeOperations<std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> > >::destruct(std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >*, std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >*) + 29 (Vector.h:241) 8 com.apple.WebKit 0x0000000103ee5670 WTF::Vector<std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >, 0ul, WTF::CrashOnOverflow, 16ul>::~Vector() + 64 9 com.apple.WebKit 0x0000000103ee1405 WTF::Vector<std::__1::unique_ptr<WebKit::WebSWServerConnection, std::__1::default_delete<WebKit::WebSWServerConnection> >, 0ul, WTF::CrashOnOverflow, 16ul>::~Vector() + 21 (Vector.h:670) 10 com.apple.WebKit 0x0000000103ee0ca2 WebKit::StorageToWebProcessConnection::didClose(IPC::Connection&) + 978 (StorageToWebProcessConnection.cpp:156) 11 com.apple.WebKit 0x00000001038f8966 IPC::Connection::connectionDidClose()::$_13::operator()() + 246 (Connection.cpp:811) 12 com.apple.WebKit 0x00000001038f87e9 WTF::Function<void ()>::CallableWrapper<IPC::Connection::connectionDidClose()::$_13>::call() + 25 (Function.h:101) 13 com.apple.JavaScriptCore 0x000000010a66060d WTF::Function<void ()>::operator()() const + 141 (Function.h:56) 14 com.apple.JavaScriptCore 0x000000010a6b4333 WTF::RunLoop::performWork() + 211 (RunLoop.cpp:107) 15 com.apple.JavaScriptCore 0x000000010a6b4c44 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38) 16 com.apple.CoreFoundation 0x00007fff37ca4470 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 1
<rdar://problem/40931680>
Created attachment 342625 [details] Patch
Created attachment 342626 [details] Patch
Comment on attachment 342626 [details] Patch Attachment 342626 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8156468 New failing tests: js/dom/JSON-stringify.html
Created attachment 342633 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 342626 [details] Patch Attachment 342626 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/8156675 New failing tests: http/tests/resourceLoadStatistics/partitioned-and-unpartitioned-cookie-with-partitioning-timeout.html
Created attachment 342636 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 342626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342626&action=review > Source/WebCore/workers/service/server/SWServer.cpp:62 > + server().unregisterConnection(*this); Can’t m_server be null here? > Source/WebCore/workers/service/server/SWServer.cpp:79 > + while (!m_connections.isEmpty()) > + (*m_connections.values().begin())->connectionInvalidated(); Why does calling SWServer::Connection::connectionInvalidated guarantee that the connection will be removed from m_connections? It seems like there is a missing call to unregisterConnection.
Created attachment 342658 [details] Patch
Comment on attachment 342658 [details] Patch Attachment 342658 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8165947 New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
Created attachment 342685 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 342685 [details] Archive of layout-test-results from ews206 for win-future Windows failure cannot be related as this change is WK2 only.
Comment on attachment 342658 [details] Patch LGTM. A question though: would it simplify the overall model if we were making SWServer owning its connections?
(In reply to youenn fablet from comment #13) > Comment on attachment 342658 [details] > Patch > > LGTM. > A question though: would it simplify the overall model if we were making > SWServer owning its connections? I have this other patch too, I implemented both. I'll upload the other one so you can compare.
Created attachment 342703 [details] Alternative (SWServer owns the connections)
Created attachment 342704 [details] Alternative (SWServer owns the connections)
Let me know which one you prefer. Both patches are about the same size.
Comment on attachment 342704 [details] Alternative (SWServer owns the connections) (In reply to Chris Dumez from comment #17) > Let me know which one you prefer. Both patches are about the same size. I slightly prefer the second patch, it seems simpler to understand in terms of who owns who. View in context: https://bugs.webkit.org/attachment.cgi?id=342704&action=review > Source/WebCore/workers/service/server/SWServer.h:150 > + Connection* connection(SWServerConnectionIdentifier identifier) { return m_connections.get(identifier); } const?
(In reply to youenn fablet from comment #18) > Comment on attachment 342704 [details] > Alternative (SWServer owns the connections) > > (In reply to Chris Dumez from comment #17) > > Let me know which one you prefer. Both patches are about the same size. > > I slightly prefer the second patch, it seems simpler to understand in terms > of who owns who. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=342704&action=review > > > Source/WebCore/workers/service/server/SWServer.h:150 > > + Connection* connection(SWServerConnectionIdentifier identifier) { return m_connections.get(identifier); } > > const? Ok, r+ ?
Comment on attachment 342704 [details] Alternative (SWServer owns the connections) Attachment 342704 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/8170319 New failing tests: imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html
Created attachment 342711 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 342716 [details] Patch
Comment on attachment 342716 [details] Patch Clearing flags on attachment: 342716 Committed r232824: <https://trac.webkit.org/changeset/232824>
All reviewed patches have been landed. Closing bug.