Bug 33008

Crashed again just now: http://build.webkit.org/results/Qt%20Linux%20Release/r53056%20(5806)/fast/workers/stress-js-execution-stderr.txt No way to know w/o symbols if that's the same back trace or not.

Another one: http://build.webkit.org/results/Qt%20Linux%20Release/r53105%20(5852)/fast/workers/stress-js-execution-stderr.txt Seems we should skip this test unless we have some idea how to fix. No sense in it keeping the Qt bot intermittently red.

I tested it with "WebKitTools/Scripts/run-webkit-tests fast/workers/stress-js-execution.html --iterations=1000", and it crashed 67 times (6.7%) It is enough frequently to skip this test until fix. I'll try to generate backtrace for it in debug mode.

Created attachment 46367 [details] valgrind dump I ran "WebKitTools/Scripts/run-webkit-tests --debug fast/workers/stress-js-execution.html --iterations 10 --valgrind", and I got 2 crashes. The stderr output attached.

ASSERTION FAILED: !methodCallLinkInfo.cachedStructure (../../../JavaScriptCore/jit/JITPropertyAccess.cpp:642 static void JSC::JIT::patchMethodCallProto(JSC::CodeBlock*, JSC::MethodCallLinkInfo&, JSC::JSFunction*, JSC::Structure*, JSC::JSObject*, JSC::ReturnAddressPtr)) This sounds like it could even be cross-platform, and quite dangerous. I think that more investigation should be performed before we can skip the test.

This part of log is hopefully a tell-tale: ==20070== Conditional jump or move depends on uninitialised value(s) ==20070== at 0x52AC901: JSC::BytecodeGenerator::createArgumentsIfNecessary() (BytecodeGenerator.cpp:1391)

Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should only be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component - Add the keyword 'Qt' to signal that it's a Qt-related bug http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

The problem is that ENABLE(JSC_MULTIPLE_THREADS) is not true on Qt, except on Mac (see JavascriptCore/wtf/Platform.h line 569). Without JSC_MULTIPLE_THREADS enabled some mutex are just not created/used (see JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp line 70). If you try the test fast/workers/stress-js-execution.html in debug it will fail in an assertion (JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp line 146). I tried run-webkit-tests --qt fast/workers/stress-js-execution.html --iterations 10000 in debug on Snow Leopard and it did not crash any time. ENABLE(JSC_MULTIPLE_THREADS) is true on Snow Leopard.

run-webkit-tests --qt fast/workers/stress-js-execution.html output: ASSERTION FAILED: isMainThread() /home/agostini/work/webkit/workdirs/wd5/Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp(146) : uint32_t WTF::<unnamed>::ARC4RandomNumberGenerator::randomNumber() Segmentation fault ASSERTION FAILED: isMainThread() /home/agostini/work/webkit/workdirs/wd5/Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp(146) : uint32_t WTF::<unnamed>::ARC4RandomNumberGenerator::randomNumber() Segmentation fault 0: /home/agostini/work/webkit/build/workdir/wd5/Release/bin/DumpRenderTree() [0x807abfd] 1: /home/agostini/work/webkit/build/workdir/wd5/Release/bin/DumpRenderTree() [0x807aea3] 2: [0xb7882400] 3: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20ed770) [0xb6596770] 4: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20ed921) [0xb6596921] 5: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20feb6b) [0xb65a7b6b] 6: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN3JSC14JSGlobalObject18JSGlobalObjectDataC2EPFvPvE+0x2c9) [0xb5a39651] 7: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore17JSDOMGlobalObject21JSDOMGlobalObjectDataC1EPNS_15DOMWrapperWorldEPFvPvE+0x26) [0xb622c74c] 8: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore19JSWorkerContextBaseC2EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_13WorkerContextEEE+0x66) [0xb622c87a] 9: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore15JSWorkerContextC2EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_13WorkerContextEEE+0x51) [0xb5903f3f] 10: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore24JSDedicatedWorkerContextC1EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_22DedicatedWorkerContextEEE+0x51) [0xb58fe785] 11: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController10initScriptEv+0x430) [0xb622e87e] 12: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController18initScriptIfNeededEv+0x2f) [0xb5a3665f] 13: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController8evaluateERKNS_16ScriptSourceCodeEPNS_11ScriptValueE+0x8b) [0xb622f16d] 14: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController8evaluateERKNS_16ScriptSourceCodeE+0xa8) [0xb622efd0] 15: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore12WorkerThread12workerThreadEv+0x183) [0xb6244ec9] 16: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore12WorkerThread17workerThreadStartEPv+0x1d) [0xb6244d3f] 17: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x210126e) [0xb65aa26e] 18: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20fba23) [0xb65a4a23] 19: /home/agostini/work/qt/build/lib/libQtCore.so.4(+0x7d0f7) [0xb32f20f7] 20: /lib/libpthread.so.0(+0x5cc9) [0xb325fcc9] 21: /lib/libc.so.6(clone+0x5e) [0xb30a069e] 0: /home/agostini/work/webkit/build/workdir/wd5/Release/bin/DumpRenderTree() [0x807abfd] 1: /home/agostini/work/webkit/build/workdir/wd5/Release/bin/DumpRenderTree() [0x807aea3] 2: [0xb7882400] 3: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20ed770) [0xb6596770] 4: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20ed921) [0xb6596921] 5: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20feb6b) [0xb65a7b6b] 6: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN3JSC14JSGlobalObject18JSGlobalObjectDataC2EPFvPvE+0x2c9) [0xb5a39651] 7: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore17JSDOMGlobalObject21JSDOMGlobalObjectDataC1EPNS_15DOMWrapperWorldEPFvPvE+0x26) [0xb622c74c] 8: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore19JSWorkerContextBaseC2EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_13WorkerContextEEE+0x66) [0xb622c87a] 9: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore15JSWorkerContextC2EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_13WorkerContextEEE+0x51) [0xb5903f3f] 10: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore24JSDedicatedWorkerContextC1EN3WTF17NonNullPassRefPtrIN3JSC9StructureEEENS1_10PassRefPtrINS_22DedicatedWorkerContextEEE+0x51) [0xb58fe785] 11: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController10initScriptEv+0x430) [0xb622e87e] 12: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController18initScriptIfNeededEv+0x2f) [0xb5a3665f] 13: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController8evaluateERKNS_16ScriptSourceCodeEPNS_11ScriptValueE+0x8b) [0xb622f16d] 14: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore22WorkerScriptController8evaluateERKNS_16ScriptSourceCodeE+0xa8) [0xb622efd0] 15: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore12WorkerThread12workerThreadEv+0x183) [0xb6244ec9] 16: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(_ZN7WebCore12WorkerThread17workerThreadStartEPv+0x1d) [0xb6244d3f] 17: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x210126e) [0xb65aa26e] 18: /home/agostini/work/webkit/build/workdir/wd5/Debug/lib/libQtWebKit.so.4(+0x20fba23) [0xb65a4a23] 19: /home/agostini/work/qt/build/lib/libQtCore.so.4(+0x7d0f7) [0xb32f20f7] 20: /lib/libpthread.so.0(+0x5cc9) [0xb325fcc9] 21: /lib/libc.so.6(clone+0x5e) [0xb30a069e] LEAK: 13 WebCoreNode LEAK: 1 Frame LEAK: 1 Page LEAK: 6 RenderObject LEAK: 112 Structure

Created attachment 84726 [details] patch

Wouldn't it be safer just to enable this for Linux and not all UNIX based system ? Would this work on all UNIX-based system (except MAC and Symbian as you noted) ? This patch un-skippes the test for all OS-es not just for Unix/Linux - this seems to imply that LayoutTests for Qt on maintained on Unix/Linux. Is that the case ? Regarding the change I would prefer not to touch the build files for this. What about simply doing the following in JavaScrioptCore/wtf/Platform.h ? +#if PLATFORM(QT) && OS(LINUX) +#define WTF_USE_PTHREADS 1 +#endif

(In reply to comment #11) > Wouldn't it be safer just to enable this for Linux and not all UNIX based system ? Would this work on all UNIX-based system (except MAC and Symbian as you noted) ? It will work in platforms where there are pthread and signals. My understanding is that it covers most of them. Symbian uses pthreads but unfortunately OpenC does not cover 100% of the API. But I am not a symbian expert and may be wrong. > > This patch un-skippes the test for all OS-es not just for Unix/Linux - this seems to imply that LayoutTests for Qt on maintained on Unix/Linux. Is that the case ? I think that the tests may be removed from skip list if they will not be a problem for the bots. > > Regarding the change I would prefer not to touch the build files for this. What about simply doing the following in JavaScrioptCore/wtf/Platform.h ? > > +#if PLATFORM(QT) && OS(LINUX) > +#define WTF_USE_PTHREADS 1 > +#endif It is not possible. It will affect many parts of the code, changing some typedefs that must point to QThread related types. Anyway, I think that how it is proposed in patch is better. I consider that to have a Qt independent define is better because it will be simple to repalce these calls by QThread calls as soon as QThread implement all that is needed.

> > Regarding the change I would prefer not to touch the build files for this. What about simply doing the following in JavaScrioptCore/wtf/Platform.h ? > > > > +#if PLATFORM(QT) && OS(LINUX) > > +#define WTF_USE_PTHREADS 1 > > +#endif > > It is not possible. It will affect many parts of the code, changing some typedefs that must point to QThread related types. Good point. I would suggest WTF_USE_QT_PTHREAD to indicate that this is a "USE type" of define and set it in Platform.h.

Created attachment 84747 [details] patch

Laszlo, is this what you asked?

> Doing the same of what is done when USE(PTHREADS) is true if USE(QT_PTHREAD) is true. This comment seems misleading - I was going to ask why we need two macros that do the same. But you are only making it do the same in MachineStackMarker, not everywhere. It sounds more like a HAVE-style macro. Or perhaps you could just define USE(PTHREADS) to true in MachineStackMarker.cpp only.

Created attachment 85338 [details] patch

(In reply to comment #16) > > Doing the same of what is done when USE(PTHREADS) is true if USE(QT_PTHREAD) is true. > > This comment seems misleading - I was going to ask why we need two macros that do the same. But you are only making it do the same in MachineStackMarker, not everywhere. Right. :) > > It sounds more like a HAVE-style macro. Or perhaps you could just define USE(PTHREADS) to true in MachineStackMarker.cpp only. This is what I did first for testing. But now I think that you are right, and it is a lot more simple just to define USE(PTHREADS) in MachineStackMarker.cpp. Last patch does it. Thanks for your comments.

Comment on attachment 85338 [details] patch Short and sweet, much better. :) r=me

Comment on attachment 85338 [details] patch Looks great. Perhaps we should add a compile-time error for !ENABLE(JSC_MULTIPLE_THREADS) in some Worker related file to catch such mistakes earlier (just be careful to not break v8).

Here goes a list of all tests that were affected by this problem and would fail from time to time due to race conditions. Most of them were not skipped and some of them could not be removed from the skipped list because they still failing due to other reasons. fast/workers/worker-timeout.html fast/workers/storage/change-version-handle-reuse-worker.html fast/workers/storage/open-database-set-empty-version-sync.html fast/workers/storage/executesql-accepts-only-one-statement-sync.html fast/workers/storage/multiple-databases-garbage-collection.html fast/workers/storage/test-authorizer.html fast/workers/storage/change-version-handle-reuse-sync.html fast/workers/storage/open-database-creation-callback-sync.html fast/workers/storage/open-database-while-transaction-in-progress.html fast/workers/storage/multiple-transactions-on-different-handles.html fast/workers/storage/change-version-sync.html fast/workers/storage/sql-data-types-sync.html fast/workers/storage/sql-exception-codes-sync.html fast/workers/storage/open-database-while-transaction-in-progress-sync.html fast/workers/storage/multiple-transactions.html fast/workers/storage/read-and-write-transactions-dont-run-together.html fast/workers/storage/execute-sql-args-worker.html fast/workers/storage/execute-sql-args-sync.html fast/workers/storage/transaction-in-transaction-sync.html fast/workers/storage/multiple-transactions-on-different-handles-sync.html fast/workers/storage/open-database-empty-version-sync.html fast/workers/storage/use-same-database-in-page-and-workers.html fast/workers/storage/use-same-database-in-page-and-workers.html fast/workers/storage/use-same-database-in-page-and-workers.html fast/workers/storage/test-authorizer-sync.html fast/workers/storage/empty-statement-sync.html fast/workers/storage/open-database-inputs-sync.html fast/workers/storage/interrupt-database.html fast/workers/shared-worker-shared.html fast/workers/shared-worker-navigator.html fast/workers/wrapper-map-gc.html fast/workers/shared-worker-replace-global-constructor.html fast/workers/shared-worker-in-iframe.html fast/workers/worker-navigator.html fast/workers/worker-replace-global-constructor.html fast/workers/worker-location.html fast/workers/worker-replace-self.html fast/workers/shared-worker-gc.html fast/workers/shared-worker-gc.html fast/workers/worker-messageport.html fast/workers/shared-worker-location.html fast/workers/worker-event-listener.html fast/workers/shared-worker-replace-self.html fast/workers/use-machine-stack.html fast/workers/worker-call.html fast/workers/shared-worker-simple.html fast/workers/worker-constructor.html fast/workers/worker-gc.html fast/workers/shared-worker-context-gc.html fast/workers/shared-worker-context-gc.html fast/workers/shared-worker-context-gc.html fast/workers/worker-script-error.html fast/workers/worker-context-gc.html http/tests/xmlhttprequest/workers/xmlhttprequest-file-not-found.html http/tests/xmlhttprequest/workers/shared-worker-xhr-file-not-found.html http/tests/xmlhttprequest/workers/abort-exception-assert.html http/tests/xmlhttprequest/workers/referer.html http/tests/xmlhttprequest/workers/shared-worker-referer.html http/tests/eventsource/workers/eventsource-simple.html http/tests/workers/text-encoding.html http/tests/workers/text-encoding.html http/tests/workers/shared-worker-importScripts.html http/tests/workers/worker-importScripts.html http/tests/workers/worker-importScriptsOnError.html

(In reply to comment #20) > (From update of attachment 85338 [details]) > Looks great. Perhaps we should add a compile-time error for !ENABLE(JSC_MULTIPLE_THREADS) in some Worker related file to catch such mistakes earlier (just be careful to not break v8). Ok. I will do it and cc you in the new bug.

Committed r80746: <http://trac.webkit.org/changeset/80746>

*** Bug 33437 has been marked as a duplicate of this bug. ***

*** Bug 33653 has been marked as a duplicate of this bug. ***

*** Bug 36633 has been marked as a duplicate of this bug. ***

Nice job Luiz. I was quite annoyed by those flaky test I could not reproduce at home. Thanks for fixing this.

fast/workers/shared-worker-frame-lifecycle.html still fails, so I had to skip it again. See https://bugs.webkit.org/show_bug.cgi?id=34281 for details.