Bug 53912

http/tests/websocket/tests/websocket-pending-activity.html crashes intermittently I'm going to find which changeset caused it. (r77705 is innocent)

Adding http/tests/websocket/tests/websocket-pending-activity.html to the Skipped list (http://trac.webkit.org/changeset/77855) made the following test http/tests/websocket/tests/websocket-protocol-ignored.html crash. I added it to the Skipped list too. (http://trac.webkit.org/changeset/77911)

Created attachment 81601 [details] crash log of http/tests/websocket/tests/websocket-protocol-ignored.html (r77906)

Created attachment 81614 [details] crash log of fast/text/international/text-combine-parser-test.html (r77156) on 64 bit

The first occurence of crashes on 64 bit appeared after http://trac.webkit.org/changeset/77151 and its buildfix http://trac.webkit.org/changeset/77156.

Created attachment 81618 [details] crash log of fast/frames/sandboxed-iframe-noscript.html (r77159) on 32 bit in debug mode fast/frames/sandboxed-iframe-noscript.html started to crash from r77159 in debug mode

I modified the importance to P1/blocker, because this bug cause flakey crashes frequently on our "Qt Linux Release" core builder bot. This flakiness made sheriffbot spamming many bugs with false positive comments like: "http://trac.webkit.org/changeset/XXXXX might have broken Qt Linux Release The following tests are not passing: ..." We don't want to rightly angry people remove Qt bots from core builders list, because after that nobody would care about Qt bots, and the build of QtWebKit would be always broken. But we can't avoid this catastrophic consequence if none of Qt/JSC expert cares about this bug as soon as possible. Any volunteer? Zoltan? Kent?

you can reproduce crashing of fast/frames/sandboxed-iframe-noscript.html easily: $ Tools/Scripts/run-webkit-tests --debug fast/frames/sandboxed-iframe-navigation-top.html fast/frames/sandboxed-iframe-navigation-windowopen.html fast/frames/sandboxed-iframe-noscript.html

gdb commands: break Source/WebCore/bridge/runtime_root.cpp:88 ignore 1 20 This RootObject will cause the crash, when it is invalidated by: #0 JSC::Bindings::RootObject::invalidate (this=0x8374e08) at ../../../Source/WebCore/bridge/runtime_root.cpp:115 #1 0xb65f6ce3 in ~ScriptController (this=0x836a4ec) at ../../../Source/WebCore/bindings/js/ScriptController.cpp:89 #2 0xb6bcc3e4 in ~Frame (this=0x836a1d0) at ../../../Source/WebCore/page/Frame.cpp:245 #3 0xb658ba71 in WTF::RefCounted<WebCore::Frame>::deref (this=0x836a1d4) at ../../../Source/JavaScriptCore/wtf/RefCounted.h:141 #4 0xb658bab3 in WTF::derefIfNotNull<WebCore::Frame> (ptr=0x836a1d0) at ../../../Source/JavaScriptCore/wtf/PassRefPtr.h:59 #5 0xb658bad9 in ~RefPtr (this=0xb270bc64) at ../../../Source/JavaScriptCore/wtf/RefPtr.h:58 #6 0xb6bfe1fe in ~Page (this=0xb270bc38) at ../../../Source/WebCore/page/Page.cpp:207 #7 0xb6f3d8ac in ~QWebPagePrivate (this=0xb270b308) at ../../../Source/WebKit/qt/Api/qwebpage.cpp:354 #8 0xb6f3dd27 in ~QWebPage (this=0xb270b128) at ../../../Source/WebKit/qt/Api/qwebpage.cpp:1945 #9 0x08068618 in ~WebPage (this=0xb270b128) at ../../../../Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:168 #10 0xb4267676 in QObjectPrivate::deleteChildren () from /usr/local/Trolltech/Qt-4.7.1/lib/libQtCore.so.4 #11 0xb426e5df in QObject::~QObject () from /usr/local/Trolltech/Qt-4.7.1/lib/libQtCore.so.4

Source/WebCore/bindings/js/ScriptController.cpp ScriptController::~ScriptController() { disconnectPlatformScriptObjects(); if (m_cacheableBindingRootObject) { m_cacheableBindingRootObject->invalidate(); m_cacheableBindingRootObject = 0; } // It's likely that destroying m_windowShells will create a lot of garbage. if (!m_windowShells.isEmpty()) { while (!m_windowShells.isEmpty()) destroyWindowShell(m_windowShells.begin()->first.get()); gcController().garbageCollectSoon(); } } Actually we call the invalidate(); here manually, and during the gcController().garbageCollectSoon() it seems we need that m_cacheableBindingRootObject must be valid. This is the issu, I have no idea how to fix it, because I don't understand the mechanics here. Could we reorder the two ifs?

http://trac.webkit.org/changeset/61062 (Robert) http://trac.webkit.org/changeset/57533 (Geoffrey) Geoffrey and Robert, you implemented the code which Zoltan mentioned in Comment #10 . Have you got any idea for fixing this bug? Or Michael, could you give us a hint? fast/frames/sandboxed-iframe-noscript.html started to crash after your patch: htttp://trac.webkit.org/changeset/76969 (Potentially Unsafe HashSet of RuntimeObject* in RootObject definition )

It may be reasonable to remove that ASSERT. It's not clear to me how you could maintain the invariant that the root object is valid throughout the lifetime of a garbage collected object, since such an object can have unpredictable lifetime.

I believe the change I'm going to make for https://bugs.webkit.org/show_bug.cgi?id=53716 will fix this as well. Basically changing Source/WebCore/bridge/jsc/BridgeJSC.cpp From: void Instance::willDestroyRuntimeObject(RuntimeObject* object) { ASSERT(m_rootObject); ASSERT(m_rootObject->isValid()); m_rootObject->removeRuntimeObject(object); m_runtimeObject.clear(object); } To: void Instance::willDestroyRuntimeObject(RuntimeObject* object) { ASSERT(m_rootObject); if (m_rootObject->isValid()) m_rootObject->removeRuntimeObject(object); m_runtimeObject.clear(object); }

(In reply to comment #13) > I believe the change I'm going to make for https://bugs.webkit.org/show_bug.cgi?id=53716 will fix this as well. Basically changing Source/WebCore/bridge/jsc/BridgeJSC.cpp Your fix resolved the "ASSERTION FAILED: m_rootObject->isValid()", thanks for it. But unfortunately the other crashes haven't been fixed yet.

Comment on attachment 81618 [details] crash log of fast/frames/sandboxed-iframe-noscript.html (r77159) on 32 bit in debug mode fixed

I made Qt Linux Release core builder happy with skipping http/tests/websocket/tests directory: http://trac.webkit.org/changeset/78039 But it is paper overing the problem, we should find the real fix. I'm going find how we can reproduce crashes on a smaller example.

Relaxing this one a bit. Also, I can't reproduce any of the issues supposedly still pending. Ossy? :3

I can't reproduce it now, so let's try to unskip websocket tests: http://trac.webkit.org/changeset/85743