Bug 33008 – [Qt] fast/workers/stress-js-execution.html is crashing on Qt bot (intermittently)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Eric Seidel (no email)]

Qt: fast/workers/stress-js-execution.html is crashing on Qt bot (intermittently)

http://build.webkit.org/results/Qt%20Linux%20Release/r52604%20(5379)/fast/workers/stress-js-execution-stderr.txt
Segmentation fault
0: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/bin/DumpRenderTree [0x8065235]
1: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/bin/DumpRenderTree [0x80655b8]
2: [0xb7fca400]
3: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff48be]
4: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff4bfd]
5: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff4c45]
6: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff4ca5]
7: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff5042]
8: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff50d9]
9: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6fe57b0]
10: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6fcbff4]
11: [0xb4305492]
12: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6f93ea7]
13: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6f9681b]
14: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6fc91c9]
15: [0xb43042a1]
16: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6f9264f]
17: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb6ff633c]
18: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb75f7a3b]
19: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb75f7de5]
20: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb7604589]
21: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb707fa7f]
22: /home/webkit/buildslave/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4 [0xb707e663]
23: /usr/local/Trolltech/Qt-4.6.0/lib/libQtCore.so.4 [0xb5fca8de]
24: /lib/i686/cmov/libpthread.so.0 [0xb5f484c0]
25: /lib/i686/cmov/libc.so.6(clone+0x5e) [0xb5da56de]

Someone with symbols for that r52604 build could tell us what those actually mean. :)

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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.

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

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.

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

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)

[Tor Arne Vestbø]

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

[Luiz Agostini]

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.

[Luiz Agostini]

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

[Luiz Agostini]

Created attachment 84726 [details]
patch

[Laszlo Gombos]

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

[Luiz Agostini]

(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.

[Laszlo Gombos]

> > 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.

[Luiz Agostini]

Created attachment 84747 [details]
patch

[Luiz Agostini]

Laszlo, is this what you asked?

[Alexey Proskuryakov]

> 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.

[Luiz Agostini]

Created attachment 85338 [details]
patch

[Luiz Agostini]

(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.

[Andreas Kling]

Comment on attachment 85338 [details]
patch

Short and sweet, much better. :) r=me

[Alexey Proskuryakov]

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).

[Luiz Agostini]

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

[Luiz Agostini]

(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.

[Luiz Agostini]

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

[Luiz Agostini]

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

[Luiz Agostini]

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

[Luiz Agostini]

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

[Benjamin Poulain]

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

[Csaba Osztrogonác]

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.