Bug 60846

Summary: ASSERT(isMainThread()) when using single threaded jsc executable
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: JavaScriptCoreAssignee: Patrick R. Gansterer <paroga>
Severity: Normal CC: abarth, ademar, ap, barraclough, commit-queue, ggaren, levin, michaeln, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 55728    
Description Flags
Patch none

Description Patrick R. Gansterer 2011-05-14 13:50:20 PDT
When I run ./jsc i get the following stack trace:

#0  0x00000000005a6b2f in WTF::(anonymous namespace)::ARC4RandomNumberGenerator::randomNumber (this=0xa23910) at ../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp:146
#1  0x00000000005a6cca in WTF::cryptographicallyRandomNumber () at ../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp:181
#2  0x00000000004fa709 in WTF::randomNumber () at ../Source/JavaScriptCore/wtf/RandomNumber.cpp:57
#3  0x0000000000415531 in JSC::JSGlobalObject::JSGlobalObject (this=0x7fffad8cc150, globalData=...) at ../Source/JavaScriptCore/runtime/JSGlobalObject.h:124
#4  0x000000000040e4b0 in GlobalObject::GlobalObject (this=0x7fffad8cc150, globalData=..., arguments=...) at ../Source/JavaScriptCore/jsc.cpp:151
#5  0x00000000004109f2 in jscmain (argc=1, argv=0x7fffffffe218, globalData=0xa1be20) at ../Source/JavaScriptCore/jsc.cpp:540
#6  0x000000000040fb27 in main (argc=1, argv=0x7fffffffe218) at ../Source/JavaScriptCore/jsc.cpp:359
Comment 1 Alexey Proskuryakov 2011-05-14 22:12:43 PDT
Main thread is largely a WebCore concept, and the few instances of it in WTF are strictly for WebCore use. This is a unique use of it in JSC, and should probably be eliminated. Thoughts?

Different ports have different requirements on how JavaScriptCore API can be used. On Mac OS X, there is no requirement that the main thread is registered somehow, or that the first use of the API is from main thread. Also on Mac OS X, there is a system notion of main thread anyway, exposed via pthread_main_np().
Comment 2 Adam Barth 2011-05-14 23:23:32 PDT
Why does the API exist in wtf if it doesn't make sense?  In any case, feel free to remove the assert.
Comment 3 Patrick R. Gansterer 2011-05-15 00:05:16 PDT
(In reply to comment #2)
> Why does the API exist in wtf if it doesn't make sense?

BTW: this the only dependency of JSC on linux to a window framework. When we remove MainThread from WTF/JSC libjavascriptcore.so can be compiled for "linux" and not for e.g. "gtk linux", "qt linux".

> In any case, feel free to remove the assert.
Are you sure it's safe to remove only the assert??? IMHO we need to use the Mutex all the time.
Comment 4 Patrick R. Gansterer 2011-05-15 00:20:01 PDT
Created attachment 93575 [details]
Comment 5 Adam Barth 2011-05-15 08:23:17 PDT
Comment on attachment 93575 [details]

We don't need the mutex if WTF_MULTIPLE_THREADS isn't defined.
Comment 6 Patrick R. Gansterer 2011-05-15 08:25:09 PDT
(In reply to comment #5)
> (From update of attachment 93575 [details])
> We don't need the mutex if WTF_MULTIPLE_THREADS isn't defined.

Can we ensure that the embedding app don't call randomNumber() while running JSC? Does the API forbid such calls?
Comment 7 Patrick R. Gansterer 2011-05-15 08:29:11 PDT
Created attachment 93580 [details]
Comment 8 Adam Barth 2011-05-15 08:47:06 PDT
Comment on attachment 93580 [details]

Lots of stuff in WTF requires WTF_MULTIPLE_THREADS to be thread-safe.
Comment 9 Michael Nordman 2011-05-25 19:04:06 PDT
Is there a reason to not land this patch?

There's a case where we trip on this assertion w/o it being a real problem.

After seeing the r+ on Patrick's patch here, I thought that problem was solved, but this is not committed yet. Is there a reason for that or would it be OK if I land it?
Comment 10 WebKit Commit Bot 2011-05-26 01:08:44 PDT
Comment on attachment 93580 [details]

Clearing flags on attachment: 93580

Committed r87369: <http://trac.webkit.org/changeset/87369>
Comment 11 WebKit Commit Bot 2011-05-26 01:08:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Ademar Reis 2011-05-26 08:58:20 PDT
Revision r87369 cherry-picked into qtwebkit-2.2 with commit ef13621 <http://gitorious.org/webkit/qtwebkit/commit/ef13621>
Comment 13 Michael Nordman 2011-05-26 13:33:18 PDT
Thank you!