[Webkit-unassigned] [Bug 125305] Incorrect usage of ThreadingOnce class.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=125305





--- Comment #5 from Brent Fulgham <bfulgham at webkit.org>  2013-12-05 12:34:59 PST ---
(From update of attachment 218528)
View in context: https://bugs.webkit.org/attachment.cgi?id=218528&action=review

I know you are basically reverting to the original code here, but I think the original code still allows multiple threads to initialize. (Geof, please correct me where I'm wrong).

We are very close to switching to VS2013, which will allow us to use "std::call_once", which might be a better cross-platform solution.

> Source/JavaScriptCore/dfg/DFGWorklist.cpp:287
> +    if (!initializedGlobalWorklist) {

It seems like this should be done using the old "check->lock mutex->check again" construct.  It seems like this logic still gives us a chance for two threads to initialize the global wordlist.

Shouldn't it be something like (pseudocode below):

static mutex initializationMutex;

Worklist* initializeGlobalWOrklist()
{
#if USE(PTHREADS)
    pthread_once(&initializeGlobalWorklistKeyOnce, initializeGlobalWorklistOnce);
 #else
    static bool initializedGlobalWorklist = false;
    if (!initializedGlobalWorklist) {
        ScopedLock locker(intializationMutex);
        if (!initializedGlobalWorklist) {
             initializeGlobalWOrklistOnce();
             initializeGlobalWorklist = true;
        }
    }

> Source/WTF/wtf/CompilationThread.cpp:52
> +    if (!initializedCompilationThreads) {

Ditto above. I think we can double-initialize with this logic.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.