Implement WTF::randomNumber in terms of WTF::cryptographicallyRandomNumber when possible
Created attachment 83987 [details] Patch
Attachment 83987 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8074086
Attachment 83987 [details] did not build on gtk: Build output: http://queues.webkit.org/results/8070180
Attachment 83987 [details] did not build on qt: Build output: http://queues.webkit.org/results/8075069
Attachment 83987 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8071142
Attachment 83987 [details] did not build on mac: Build output: http://queues.webkit.org/results/8024889
Attachment 83987 [details] did not build on win: Build output: http://queues.webkit.org/results/8070191
Attachment 83987 [details] did not build on win: Build output: http://queues.webkit.org/results/8075089
Comment on attachment 83987 [details] Patch Turns out coding is hard. Let's go shopping!
Created attachment 83996 [details] Patch
Attachment 83996 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8075091
Attachment 83996 [details] did not build on gtk: Build output: http://queues.webkit.org/results/8073143
Attachment 83996 [details] did not build on qt: Build output: http://queues.webkit.org/results/8024891
Comment on attachment 83996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83996&action=review > Source/JavaScriptCore/wtf/RandomNumber.cpp:31 > +#include "cryptographicallyRandomNumber.h" Probably your caps here.
Created attachment 84003 [details] Patch
Comment on attachment 84003 [details] Patch LGTM.
Comment on attachment 84003 [details] Patch Clearing flags on attachment: 84003 Committed r79835: <http://trac.webkit.org/changeset/79835>
All reviewed patches have been landed. Closing bug.
It seems this patch broke the jscore-tests in the gtk debug bots, it fails in an assertion: ASSERTION FAILED: isMainThread()<br> ../../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp(146) : uint32_t WTF::<unnamed>::ARC4RandomNumberGenerator::randomNumber()<br> </tt><br> http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Debug/builds/14100/steps/jscore-test/logs/results
More information about the assertion failure: Thread 1 (Thread 11402): #0 0x00007f7c7de5d175 in WTF::(anonymous namespace)::ARC4RandomNumberGenerator::randomNumber (this=0xd70340) at ../../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp:146 #1 0x00007f7c7de5d2ef in WTF::cryptographicallyRandomNumber () at ../../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp:181 #2 0x00007f7c7de6b171 in WTF::randomNumber () at ../../Source/JavaScriptCore/wtf/RandomNumber.cpp:57 #3 0x00007f7c7cfff3d9 in JSC::JSGlobalObject::JSGlobalObjectData::JSGlobalObjectData (this=0x7f7c6c009780, destructor=0x7f7c7cffa2d6 <WebCore::JSDOMGlobalObject::destroyJSDOMGlobalObjectData(void*)>) at ../../Source/JavaScriptCore/runtime/JSGlobalObject.h:73 #4 0x00007f7c7d03d83b in WebCore::JSDOMGlobalObject::JSDOMGlobalObjectData::JSDOMGlobalObjectData (this=0x7f7c6c009780, world=0x7f7c6c005ff0, destructor=0x7f7c7cffa2d6 <WebCore::JSDOMGlobalObject::destroyJSDOMGlobalObjectData(void*)>) at ../../Source/WebCore/bindings/js/JSDOMGlobalObject.h:82 #5 0x00007f7c7d03d34e in WebCore::JSWorkerContextBase::JSWorkerContextBase (this=0x7f7c2acc10d0, structure=..., impl=...) at ../../Source/WebCore/bindings/js/JSWorkerContextBase.cpp:49 #6 0x00007f7c7dbba6af in WebCore::JSWorkerContext::JSWorkerContext (this=0x7f7c2acc10d0, structure=..., impl=...) at DerivedSources/WebCore/JSWorkerContext.cpp:163 #7 0x00007f7c7db83dbb in WebCore::JSSharedWorkerContext::JSSharedWorkerContext (this=0x7f7c2acc10d0, structure=..., impl=...) at DerivedSources/WebCore/JSSharedWorkerContext.cpp:88 #8 0x00007f7c7d072fdb in WebCore::WorkerScriptController::initScript (this=0x778190) at ../../Source/WebCore/bindings/js/WorkerScriptController.cpp:94 #9 0x00007f7c7cffb20c in WebCore::WorkerScriptController::initScriptIfNeeded (this=0x778190) at ../../Source/WebCore/bindings/js/WorkerScriptController.h:74 #10 0x00007f7c7d073323 in WebCore::WorkerScriptController::evaluate (this=0x778190, sourceCode=..., exception=0x7f7c2b946ad0) at ../../Source/WebCore/bindings/js/WorkerScriptController.cpp:125 #11 0x00007f7c7d0731dc in WebCore::WorkerScriptController::evaluate (this=0x778190, sourceCode=...) at ../../Source/WebCore/bindings/js/WorkerScriptController.cpp:109 #12 0x00007f7c7d9aabe5 in WebCore::WorkerThread::workerThread (this=0xd523d0) at ../../Source/WebCore/workers/WorkerThread.cpp:135 #13 0x00007f7c7d9aaa40 in WebCore::WorkerThread::workerThreadStart (thread=0xd523d0) at ../../Source/WebCore/workers/WorkerThread.cpp:118 #14 0x00007f7c7de78d11 in WTF::threadEntryPoint (contextData=0x778190) at ../../Source/JavaScriptCore/wtf/Threading.cpp:67 #15 0x00007f7c78f158ba in start_thread () from /lib/libpthread.so.0 #16 0x00007f7c78c7d02d in clone () from /lib/libc.so.6 http://webkit-bots.igalia.com/amd64/svn_79853.core-when_1298894990-_-who_DumpRenderTree-_-why_11.trace.html
It would appear that the cryptographically secure source is not threadsafe, abarth?
It's supposed to be thread-safe. Maybe I screwed up the ifdefs? I can look at this this afternoon. Perhaps the threading ifdefs on gtk are different than on Mac?
http://trac.webkit.org/changeset/79835 might have broken GTK Linux 32-bit Debug
There are similar assertions on Qt debug bots: ASSERTION FAILED: isMainThread() ../../../Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp(146) : uint32_t WTF::<unnamed>::ARC4RandomNumberGenerator::randomNumber() All fast/workers tests assert. Reopen to fix the bug.
> There are similar assertions on Qt debug bots: There's been a bunch of discussion on the GTK version of this bug. Basically, ENABLE(WORKERS) needs to imply ENABLE(JSC_MULTIPLE_THREADS) or else you'll have a bunch of race conditions. This code just ASSERTS that you don't have race conditions, which is catching the underlying error.
FYI yesterday we landed http://trac.webkit.org/changeset/79952 trying to fix this for the ports using PTHREAD, we had an old bug with a outdated patch and we reviewed it to have an initial solution that we have to review because the situation it is still not clear.
Zoltan and/or Gabor, could check ( and maybe fix :) ) the Qt asserts?
(In reply to comment #25) > > There are similar assertions on Qt debug bots: > > There's been a bunch of discussion on the GTK version of this bug. Basically, ENABLE(WORKERS) needs to imply ENABLE(JSC_MULTIPLE_THREADS) or else you'll have a bunch of race conditions. This code just ASSERTS that you don't have race conditions, which is catching the underlying error. Yeah, yesterday we still did not add this condition to the patch we landed because we did not want to cause more trouble to other ports with this initial patch if they were not detecting it, but basically it seems we should add something like this: diff --git a/Source/JavaScriptCore/wtf/Platform.h b/Source/JavaScriptCore/wtf/Platform.h index 2f1f592..f554756 100644 --- a/Source/JavaScriptCore/wtf/Platform.h +++ b/Source/JavaScriptCore/wtf/Platform.h @@ -563,7 +563,7 @@ #define WTF_USE_MERSENNE_TWISTER_19937 1 #endif -#if (PLATFORM(GTK) || PLATFORM(IOS) || PLATFORM(MAC) || PLATFORM(WIN) || (PLATFORM(QT) && OS(DARWIN) && !ENABLE(SINGLE_THREADED))) && !defined(ENABLE_JSC_MULTIPLE_THREADS) +#if (ENABLE(WORKERS) || PLATFORM(GTK) || PLATFORM(IOS) || PLATFORM(MAC) || PLATFORM(WIN) || (PLATFORM(QT) && OS(DARWIN) && !ENABLE(SINGLE_THREADED))) && !defined(ENABLE_JSC_MULTIPLE_THREADS) #define ENABLE_JSC_MULTIPLE_THREADS 1 #endif Ossy could you check if that fixes the problem? Thanks.
Created attachment 84211 [details] crashlog for shared-worker-constructor I tried the patch in Comment #28, but tests still crash ( but don't assert :) ) I attached one of the crash logs.
The top part of that call-stack doesn't have symbols, but that crash is somewhat mysterious to me.
It seems http://trac.webkit.org/changeset/80746 solved the crashes on Qt bots. (https://bugs.webkit.org/show_bug.cgi?id=33008)