53716 2011-02-03 14:09:33 -0800 JSC::Bindings m_rootObject->isValid() assert fails when running layout tests 2011-02-08 15:57:47 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar, LayoutTestFailure P2 Normal --- 1 mihaip webkit-unassigned abarth ap aroben darin eric ggaren mario mrobinson msaboff ossy sam webkit.review.bot oldest_to_newest 344993 0 mihaip 2011-02-03 14:09:33 -0800 To reproduce, with a debug build run: run-webkit-tests fast/events/recorded-keydown-event.html fast/events/tabindex-focus-blur-all.html Output is: FlateDecode: decoding error: incorrect data check. ASSERTION FAILED: m_rootObject->isValid() (/Users/mihaip/Developer/source/chromium1/src/third_party/WebKit/Source/WebCore/bridge/jsc/BridgeJSC.cpp:109 void JSC::Bindings::Instance::willDestroyRuntimeObject(JSC::Bindings::RuntimeObject*)) -> JSC::Bindings::Instance::willDestroyRuntimeObject(JSC::Bindings::RuntimeObject*) -> JSC::Bindings::RuntimeObject::~RuntimeObject() -> WebKit::ProxyRuntimeObject::~ProxyRuntimeObject() -> JSC::MarkedSpace::allocate(unsigned long) -> JSC::Heap::allocate(unsigned long) -> JSC::JSCell::operator new(unsigned long, JSC::ExecState*) -> JSC::BooleanPrototype::BooleanPrototype(JSC::ExecState*, JSC::JSGlobalObject*, WTF::NonNullPassRefPtr<JSC::Structure>, JSC::Structure*) -> JSC::JSGlobalObject::reset(JSC::JSValue) -> JSC::JSGlobalObject::init(JSC::JSObject*) -> JSC::JSGlobalObject::JSGlobalObject(WTF::NonNullPassRefPtr<JSC::Structure>, JSC::JSGlobalObject::JSGlobalObjectData*, JSC::JSObject*) -> WebCore::JSDOMGlobalObject::JSDOMGlobalObject(WTF::NonNullPassRefPtr<JSC::Structure>, WebCore::JSDOMGlobalObject::JSDOMGlobalObjectData*, JSC::JSObject*) -> WebCore::JSDOMWindowBase::JSDOMWindowBase(WTF::NonNullPassRefPtr<JSC::Structure>, WTF::PassRefPtr<WebCore::DOMWindow>, WebCore::JSDOMWindowShell*) -> WebCore::JSDOMWindow::JSDOMWindow(WTF::NonNullPassRefPtr<JSC::Structure>, WTF::PassRefPtr<WebCore::DOMWindow>, WebCore::JSDOMWindowShell*) -> WebCore::JSDOMWindowShell::setWindow(WTF::PassRefPtr<WebCore::DOMWindow>) -> WebCore::ScriptController::clearWindowShell(bool) -> WebCore::Frame::pageDestroyed() -> WebCore::FrameLoader::closeAndRemoveChild(WebCore::Frame*) -> WebCore::FrameLoader::detachFromParent() -> WebCore::FrameLoader::detachChildren() -> WebCore::FrameLoader::detachFromParent() -> WebCore::FrameLoader::frameDetached() -> WebCore::HTMLFrameOwnerElement::willRemove() -> WebCore::HTMLFrameElementBase::willRemove() -> WebCore::ContainerNode::willRemove() -> WebCore::ContainerNode::willRemove() -> WebCore::willRemoveChildren(WebCore::ContainerNode*) -> WebCore::ContainerNode::removeChildren() -> WebCore::Document::implicitOpen() -> WebCore::Document::open(WebCore::Document*) -> WebCore::Document::write(WebCore::SegmentedString const&, WebCore::Document*) -> WebCore::documentWrite(JSC::ExecState*, WebCore::HTMLDocument*, WebCore::NewlineRequirement) (that assert was added by http://trac.webkit.org/changeset/48513) 345003 1 aroben 2011-02-03 14:16:28 -0800 This is happening on Apple's Windows port, too: http://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r77529%20(24884)/CrashLog_0d74_2011-02-03_13-15-39-812.txt 345005 2 aroben 2011-02-03 14:19:34 -0800 <rdar://problem/8955878> 346036 3 mrobinson 2011-02-04 20:13:10 -0800 Here's another stack trace from the 64-bit GTK+ debug bot: warning: Can't read pathname for load map: Input/output error. Core was generated by `/home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/Programs/DumpR'. Program terminated with signal 11, Segmentation fault. #0 0x00007f8a22fff8e0 in JSC::Bindings::Instance::willDestroyRuntimeObject (this=0x78513c0, object=0x7f8a146f5e40) at ../../Source/WebCore/bridge/jsc/BridgeJSC.cpp:109 109 ASSERT(m_rootObject->isValid()); #0 0x00007f8a22fff8e0 in JSC::Bindings::Instance::willDestroyRuntimeObject (this=0x78513c0, object=0x7f8a146f5e40) at ../../Source/WebCore/bridge/jsc/BridgeJSC.cpp:109 #1 0x00007f8a2300a014 in JSC::Bindings::RuntimeObject::~RuntimeObject (this=0x7f8a146f5e40, __in_chrg=<value optimized out>) at ../../Source/WebCore/bridge/runtime_object.cpp:58 #2 0x00007f8a22ff93c4 in JSC::Bindings::CRuntimeObject::~CRuntimeObject (this=0x7f8a146f5e40, __in_chrg=<value optimized out>) at ../../Source/WebCore/bridge/c/CRuntimeObject.cpp:45 #3 0x00007f8a23d263df in JSC::MarkedSpace::sweep (this=0x28df4e8) at ../../Source/JavaScriptCore/runtime/MarkedSpace.cpp:272 #4 0x00007f8a23d2821f in JSC::Heap::reset (this=0x28df4e0, sweepToggle=JSC::Heap::DoSweep) at ../../Source/JavaScriptCore/runtime/Heap.cpp:387 #5 0x00007f8a23d2816b in JSC::Heap::collectAllGarbage (this=0x28df4e0) at ../../Source/JavaScriptCore/runtime/Heap.cpp:371 #6 0x00007f8a22f4f4a1 in WebCore::collect () at ../../Source/WebCore/bindings/js/GCController.cpp:46 #7 0x00007f8a22f4f5f4 in WebCore::GCController::gcTimerFired (this=0x2909940) at ../../Source/WebCore/bindings/js/GCController.cpp:69 #8 0x00007f8a22f4f80e in WebCore::Timer<WebCore::GCController>::fired (this=0x2909940) at ../../Source/WebCore/platform/Timer.h:99 #9 0x00007f8a235f3e38 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x22fabb0) at ../../Source/WebCore/platform/ThreadTimers.cpp:112 #10 0x00007f8a235f3d6f in WebCore::ThreadTimers::sharedTimerFired () at ../../Source/WebCore/platform/ThreadTimers.cpp:90 #11 0x00007f8a22df83e6 in WebCore::timeout_cb () at ../../Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49 #12 0x00007f8a20023dbb in g_timeout_dispatch (source=0x8cc0080, callback=0, user_data=0x7f8a1ee99e00) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3877 #13 0x00007f8a20023362 in g_main_dispatch (context=0x224d1e0) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:2440 #14 g_main_context_dispatch (context=0x224d1e0) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3013 #15 0x00007f8a20027a28 in g_main_context_iterate (context=0x224d1e0, block=<value optimized out>, dispatch=<value optimized out>, self=<value optimized out>) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3091 #16 0x00007f8a20027f35 in g_main_loop_run (loop=0x7f8a10377880) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3299 #17 0x00007f8a21f5c657 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #18 0x000000000041cd22 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:654 #19 0x000000000041c3f2 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:468 #20 0x000000000041e325 in main (argc=2, argv=0x7fff141a8c38) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1095 346138 4 aroben 2011-02-05 07:20:14 -0800 We should skip these tests for now. 346162 5 mrobinson 2011-02-05 10:34:53 -0800 Committed r77747: <http://trac.webkit.org/changeset/77747> 347361 6 mrobinson 2011-02-08 07:39:35 -0800 *** Bug 53991 has been marked as a duplicate of this bug. *** 347641 7 81683 msaboff 2011-02-08 13:35:00 -0800 Created attachment 81683 Patch to change failing ASSERT to conditionally remove RuntimeObject There is a race where a RuntimeObject has been marked for garbage collection but not reclaimed. When RootObject::invalidate() is called for a RootObject containing a GC'ed RuntimeObject, the invalidate call for the RuntimeObject will not be called (due to the m_runtimeObject.isValid(it) check). The m_runtimeObjects WeakGCMap will be cleared and the m_valid flag will be cleared. Later, when the RuntimeObject's dtor is called, it will clean up the related Instance object which was leading to the ASSERTion failure. The code has been changed to make the removal of the runtime object conditional on m_rootObject being valid. If it isn't valid, the RuntimeObject has already been removed. 347643 8 81683 darin 2011-02-08 13:38:33 -0800 Comment on attachment 81683 Patch to change failing ASSERT to conditionally remove RuntimeObject View in context: https://bugs.webkit.org/attachment.cgi?id=81683&action=review review+ but I would like the patch better if it didn’t do the same thing twice. > Source/WebCore/bridge/jsc/BridgeJSC.cpp:110 > - ASSERT(m_rootObject->isValid()); > - m_rootObject->removeRuntimeObject(object); > + if (m_rootObject->isValid()) > + m_rootObject->removeRuntimeObject(object); Good to remove the assertion, but why check isValid here since you already added the check to removeRuntimeObject? 347659 9 msaboff 2011-02-08 14:03:47 -0800 I added the isValid check in RootObject::removeRuntimeObject() so that a future caller of the method other than Instance:: willDestroyRuntimeObject() won't expose this race issue. The check in willDestroyRuntimeObject() makes it so that RootObject::removeRuntimeObject() won't be called. Therefore the check is only made once in the current case. 347661 10 darin 2011-02-08 14:06:06 -0800 (In reply to comment #9) > The check in willDestroyRuntimeObject() makes it so that RootObject::removeRuntimeObject() won't be called. Therefore the check is only made once in the current case. If invalid, the check is made once. If valid, the check is made twice. No need for double checks here. 347726 11 msaboff 2011-02-08 15:03:38 -0800 Made the change suggested by the reviewer before checking in. 347728 12 msaboff 2011-02-08 15:04:22 -0800 *** Bug 54022 has been marked as a duplicate of this bug. *** 347732 13 msaboff 2011-02-08 15:10:32 -0800 Committed r77979: <http://trac.webkit.org/changeset/77979> 347734 14 mrobinson 2011-02-08 15:13:21 -0800 (In reply to comment #13) > Committed r77979: <http://trac.webkit.org/changeset/77979> From your commit: Index: trunk/LayoutTests/platform/gtk/Skipped =================================================================== --- a/trunk/LayoutTests/platform/gtk/Skipped +++ b/trunk/LayoutTests/platform/gtk/Skipped @@ -4643,6 +4643,4 @@ http/tests/websocket/tests/cross-origin.html -# These tests fail ASSERT(m_rootObject->isValid()); on the GTK+ debug bots. -# It should be unskipped when https://bugs.webkit.org/show_bug.cgi?id=53716 is fixed. -fast/frames/set-parent-src-synchronously.html -fast/frames/sandboxed-iframe-storage.html +# GTK+ DRT should support enabling and disabling the icon database +http/tests/inspector/resource-har-conversion.html I think you mistakenly reskipped a test I just unskipped. Looks like a merge issue. I'll unskip again. 347766 15 webkit.review.bot 2011-02-08 15:57:47 -0800 http://trac.webkit.org/changeset/77979 might have broken Qt Linux Release The following tests are not passing: fast/forms/input-maxlength-unsupported.html fast/forms/input-number-keyoperation.html fast/forms/input-number-unacceptable-style.html 81683 2011-02-08 13:35:00 -0800 2011-02-08 13:38:33 -0800 Patch to change failing ASSERT to conditionally remove RuntimeObject 53716.patch text/plain 3565 msaboff SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDc3OTY0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTEtMDItMDggIE1pY2hhZWwg U2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CisKKyAgICAgICAgSlNDOjpCaW5kaW5ncyBtX3Jv b3RPYmplY3QtPmlzVmFsaWQoKSBhc3NlcnQgZmFpbHMgd2hlbiBydW5uaW5nIGxheW91dCB0ZXN0 cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTM3MTYK KworICAgICAgICBDaGFuZ2VkIEFTU0VSVCB0byBiZSBhbiBpZiBmb3IgdGhlIGNhc2Ugd2hlcmUg dGhlIFJ1bnRpbWVPYmplY3Qgd2FzCisgICAgICAgIEdDJ2VkIGJlZm9yZSBSb290T2JqZWN0Ojpp bnZhbGlkYXRlIGlzIGNhbGxlZC4gIEluIHRoYXQgY2FzZSB0aGVyZSBpcyAKKyAgICAgICAgbm8g bmVlZCB0byByZW1vdmUgdGhlIFJ1bnRpbWVPYmplY3QgZnJvbSB0aGUgUm9vdE9iamVjdC4gIFRo ZSBpc1ZhbGlkKCkKKyAgICAgICAgY2FsbCB3aWxsIGJlIGZhbHNlIGluIHRoaXMgY2FzZSwgYnV0 IHRoZSBSdW50aW1lT2JqZWN0IGhhcyBhbHJlYWR5CisgICAgICAgIGJlZW4gcmVtb3ZlZCBmcm9t IHRoZSBSb290T2JqZWN0LiAgQWRkZWQgc2ltaWxhciBkZWZlbnNpdmUgY29kZQorICAgICAgICBp biBSb290T2JqZWN0OjpyZW1vdmVSdW50aW1lT2JqZWN0KCkuCisKKyAgICAgICAgKiBicmlkZ2Uv anNjL0JyaWRnZUpTQy5jcHA6CisgICAgICAgIChKU0M6OkJpbmRpbmdzOjpJbnN0YW5jZTo6d2ls bERlc3Ryb3lSdW50aW1lT2JqZWN0KToKKyAgICAgICAgKiBicmlkZ2UvcnVudGltZV9yb290LmNw cDoKKyAgICAgICAgKEpTQzo6QmluZGluZ3M6OlJvb3RPYmplY3Q6OnJlbW92ZVJ1bnRpbWVPYmpl Y3QpOgorCiAyMDExLTAyLTA4ICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9icmlk Z2UvcnVudGltZV9yb290LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9icmlkZ2Uv cnVudGltZV9yb290LmNwcAkocmV2aXNpb24gNzc5NDQpCisrKyBTb3VyY2UvV2ViQ29yZS9icmlk Z2UvcnVudGltZV9yb290LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTg1LDcgKzE4NSw5IEBAIHZv aWQgUm9vdE9iamVjdDo6YWRkUnVudGltZU9iamVjdChSdW50aW0KIAogdm9pZCBSb290T2JqZWN0 OjpyZW1vdmVSdW50aW1lT2JqZWN0KFJ1bnRpbWVPYmplY3QqIG9iamVjdCkKIHsKLSAgICBBU1NF UlQobV9pc1ZhbGlkKTsKKyAgICBpZiAoIW1faXNWYWxpZCkKKyAgICAgICAgcmV0dXJuOworCiAg ICAgQVNTRVJUKG1fcnVudGltZU9iamVjdHMudW5jaGVja2VkR2V0KG9iamVjdCkpOwogCiAgICAg bV9ydW50aW1lT2JqZWN0cy50YWtlKG9iamVjdCk7CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9icmlk Z2UvanNjL0JyaWRnZUpTQy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvYnJpZGdl L2pzYy9CcmlkZ2VKU0MuY3BwCShyZXZpc2lvbiA3Nzk0NCkKKysrIFNvdXJjZS9XZWJDb3JlL2Jy aWRnZS9qc2MvQnJpZGdlSlNDLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTA2LDggKzEwNiw4IEBA IFJ1bnRpbWVPYmplY3QqIEluc3RhbmNlOjpuZXdSdW50aW1lT2JqZWMKIHZvaWQgSW5zdGFuY2U6 OndpbGxEZXN0cm95UnVudGltZU9iamVjdChSdW50aW1lT2JqZWN0KiBvYmplY3QpCiB7CiAgICAg QVNTRVJUKG1fcm9vdE9iamVjdCk7Ci0gICAgQVNTRVJUKG1fcm9vdE9iamVjdC0+aXNWYWxpZCgp KTsKLSAgICBtX3Jvb3RPYmplY3QtPnJlbW92ZVJ1bnRpbWVPYmplY3Qob2JqZWN0KTsKKyAgICBp ZiAobV9yb290T2JqZWN0LT5pc1ZhbGlkKCkpCisgICAgICAgIG1fcm9vdE9iamVjdC0+cmVtb3Zl UnVudGltZU9iamVjdChvYmplY3QpOwogICAgIG1fcnVudGltZU9iamVjdC5jbGVhcihvYmplY3Qp OwogfQogCkluZGV4OiBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0 VGVzdHMvQ2hhbmdlTG9nCShyZXZpc2lvbiA3Nzk2NCkKKysrIExheW91dFRlc3RzL0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDExLTAyLTA4ICBNaWNoYWVsIFNh Ym9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIEpTQzo6QmluZGluZ3MgbV9yb290T2JqZWN0LT5pc1ZhbGlkKCkg YXNzZXJ0IGZhaWxzIHdoZW4gcnVubmluZyBsYXlvdXQgdGVzdHMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTUzNzE2CisKKyAgICAgICAgUmUtZW5hYmxl ZCB0ZXN0cyB0aGF0IGFyZSBmaXhlZCB3aXRoIHRoZSBjb3JyZXNwb25kaW5nIGNoYW5nZSBpbiAK KyAgICAgICAgV2ViQ29yZS4KKworICAgICAgICAqIHBsYXRmb3JtL2d0ay9Ta2lwcGVkOgorCiAy MDExLTAyLTA4ICBYaWFvbWVpIEppICA8eGppQGNocm9taXVtLm9yZz4KIAogICAgICAgICBVbnJl dmlld2VkLiBGaXggdHlwbyBpbiByZWJhc2VsaW5lIGluIHI3Nzk2MS4KSW5kZXg6IExheW91dFRl c3RzL3BsYXRmb3JtL2d0ay9Ta2lwcGVkCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL3BsYXRm b3JtL2d0ay9Ta2lwcGVkCShyZXZpc2lvbiA3Nzk0NCkKKysrIExheW91dFRlc3RzL3BsYXRmb3Jt L2d0ay9Ta2lwcGVkCSh3b3JraW5nIGNvcHkpCkBAIC00NjY3LDggKzQ2NjcsMyBAQCBodHRwL3Rl c3RzL3dlYnNvY2tldC90ZXN0cy91cmwtd2l0aC1jcmVkCiBodHRwL3Rlc3RzL3dlYnNvY2tldC90 ZXN0cy91cmwtd2l0aC1lbXB0eS1xdWVyeS5odG1sCiBodHRwL3Rlc3RzL3dlYnNvY2tldC90ZXN0 cy9zaW1wbGUuaHRtbAogaHR0cC90ZXN0cy93ZWJzb2NrZXQvdGVzdHMvY3Jvc3Mtb3JpZ2luLmh0 bWwKLQotIyBUaGVzZSB0ZXN0cyBmYWlsIEFTU0VSVChtX3Jvb3RPYmplY3QtPmlzVmFsaWQoKSk7 IG9uIHRoZSBHVEsrIGRlYnVnIGJvdHMuCi0jIEl0IHNob3VsZCBiZSB1bnNraXBwZWQgd2hlbiBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTM3MTYgaXMgZml4ZWQuCi1m YXN0L2ZyYW1lcy9zZXQtcGFyZW50LXNyYy1zeW5jaHJvbm91c2x5Lmh0bWwKLWZhc3QvZnJhbWVz L3NhbmRib3hlZC1pZnJhbWUtc3RvcmFnZS5odG1sCg==