Bug 55728 - [fileapi] Worker File API calls that create Blobs fail in debug builds due to random number generator thread assertion
Summary: [fileapi] Worker File API calls that create Blobs fail in debug builds due to...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 59144 60846
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-03 17:12 PST by Adam Klein
Modified: 2011-05-26 11:07 PDT (History)
15 users (show)

See Also:


Attachments
Patch (1.18 KB, patch)
2011-04-21 11:04 PDT, Adam Klein
no flags Details | Formatted Diff | Diff
fastMalloc windows fix (2.55 KB, patch)
2011-05-05 17:54 PDT, Michael Nordman
no flags Details | Formatted Diff | Diff
fastMalloc windows fix (2.59 KB, patch)
2011-05-05 18:43 PDT, Michael Nordman
no flags Details | Formatted Diff | Diff
merged patches (3.41 KB, patch)
2011-05-09 13:19 PDT, Michael Nordman
levin: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (209.99 KB, application/zip)
2011-05-09 14:13 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2011-03-03 17:12:51 PST
File API calls like FileEntry.file() fail when run in Worker threads due to an ASSERT(isMainThread()) in WTF::ARC4RandomNumberGenerator::randomNumber(), which is called by the UUID code utilized by BlobURL.

It's not clear to me whether the fix is to enable JSC_MULTIPLE_THREADS (which would wrap accesses in a Mutex) or to use a different RNG.
Comment 1 Eric U. 2011-03-03 17:15:02 PST
Adding Jian Li, who may know.
Comment 2 Jian Li 2011-03-03 17:34:31 PST
(In reply to comment #0)
> File API calls like FileEntry.file() fail when run in Worker threads due to an ASSERT(isMainThread()) in WTF::ARC4RandomNumberGenerator::randomNumber(), which is called by the UUID code utilized by BlobURL.
> 
> It's not clear to me whether the fix is to enable JSC_MULTIPLE_THREADS (which would wrap accesses in a Mutex) or to use a different RNG.

Which build do you see this problem? Chromium? OS? Could you please also attach the repro script?
Comment 3 Adam Klein 2011-03-03 17:39:30 PST
See http://codereview.chromium.org/6609040/ for where this came up; I was enabling some previously disabled tests (in fact, they've always been disabled).  And here's the trybot failure:

http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/15118/steps/ui_tests/logs/stdio
Comment 4 Kinuko Yasuda 2011-03-03 17:45:51 PST
(In reply to comment #3)
> See http://codereview.chromium.org/6609040/ for where this came up; I was enabling some previously disabled tests (in fact, they've always been disabled).  And here's the trybot failure:
> 
> http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/15118/steps/ui_tests/logs/stdio

You can also repro this by running chromium ToT build like this:
$ path/to/chrome --allow-file-access-from-files files:///path/to/LayoutTests/fast/filesystem/workers/file-from-file-entry.html
Comment 5 Kinuko Yasuda 2011-03-03 17:51:06 PST
Call stack:

#0  0x0000000003363ac7 in WTF::(anonymous namespace)::ARC4RandomNumberGenerator:
:randomNumber (this=0x7fb96afb7ea0)
    at third_party/WebKit/Source/JavaScriptCore/wtf/CryptographicallyRandomNumbe
r.cpp:146
#1  0x0000000003363c2a in WTF::cryptographicallyRandomNumber ()
    at third_party/WebKit/Source/JavaScriptCore/wtf/CryptographicallyRandomNumber.cpp:181
#2  0x0000000003365ad1 in WTF::randomNumber ()
    at third_party/WebKit/Source/JavaScriptCore/wtf/RandomNumber.cpp:57
#3  0x00000000033d2de8 in WebCore::createCanonicalUUIDString ()
    at third_party/WebKit/Source/WebCore/platform/UUID.cpp:99
#4  0x00000000029f29e7 in WebCore::BlobURL::createBlobURL (originString=...)
    at third_party/WebKit/Source/WebCore/fileapi/BlobURL.cpp:79
#5  0x00000000029f2767 in WebCore::BlobURL::createInternalURL ()
    at third_party/WebKit/Source/WebCore/fileapi/BlobURL.cpp:52
#6  0x00000000029f147a in WebCore::Blob::Blob (this=0x7fb96af5f460, 
    blobData=..., size=-1)
    at third_party/WebKit/Source/WebCore/fileapi/Blob.cpp:47
#7  0x00000000029fd599 in WebCore::File::File (this=0x7fb96af5f460, path=...)
    at third_party/WebKit/Source/WebCore/fileapi/File.cpp:52
#8  0x00000000023fb149 in WebCore::File::create (path=...)
    at third_party/WebKit/Source/WebCore/fileapi/File.h:42
#9  0x00000000029f6ebb in WebCore::DOMFileSystem::createFile (
    this=0x7fb95f3633c0, fileEntry=0x7fb95f361db0, successCallback=...)
    at third_party/WebKit/Source/WebCore/fileapi/DOMFileSystem.cpp:117
#10 0x00000000029fdcd8 in WebCore::FileEntry::file (this=0x7fb95f361db0, 
    successCallback=..., errorCallback=...)
    at third_party/WebKit/Source/WebCore/fileapi/FileEntry.cpp:56
Comment 6 Jian Li 2011-03-04 13:06:20 PST
This assert only occurs in Linux chromium build. When we create a blob, we need to call the WTF random number generation function to create a blob URL. When a blob is created within a worker thread, we will hit an assert in wtf function that complains about the current thread not being main thread. 

I think the fix is to enable JSC_MULTIPLE_THREADS in chromium build. Currently the web worker is running in a worker process which contains only one WebKit thread. So disabling JSC_MULTIPLE_THREADS is fine. Since we're going to change it to run the worker in a worker thread of the same renderer process (per dimich's V8 work), I think it is the right time for us to enable this flag. Adam and Darin, how do you think?
Comment 7 Adam Klein 2011-03-22 12:34:23 PDT
A quick codesearch finds lots of references to JSC_MULTIPLE_THREADS, so I'd be a little worried about turning it on without doing some profiling.  Coming at this from a different direction, how much do we care about UUIDs being cryptographically random?  We can't care that much, since we create them using randomNumber() on Mac, which doesn't seem to use OS_RANDOMNESS.  Could we just switch the UUID implementation on Chromium/Linux to use something less secure but already threadsafe?
Comment 8 Adam Klein 2011-03-22 12:38:58 PDT
Adding levin, in case he has thoughts about JSC_MULTIPLE_THREADS.
Comment 9 David Levin 2011-03-22 13:02:30 PDT
(In reply to comment #8)
> Adding levin, in case he has thoughts about JSC_MULTIPLE_THREADS.

I agree with Jian that we should enable this. The majority of usage in JavaScriptCore !wtf which is really about making jsc multithreaded but it has propagated outside of that directory to other places that need to be guarded as well. Given that we will do worker and isolates soon, we'll need to flip this switch.
Comment 10 Adam Klein 2011-04-21 11:04:17 PDT
Created attachment 90558 [details]
Patch
Comment 11 WebKit Commit Bot 2011-04-21 14:40:02 PDT
Comment on attachment 90558 [details]
Patch

Clearing flags on attachment: 90558

Committed r84548: <http://trac.webkit.org/changeset/84548>
Comment 12 WebKit Commit Bot 2011-04-21 14:40:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Review Bot 2011-04-21 15:14:20 PDT
http://trac.webkit.org/changeset/84548 might have broken Chromium Win Release
Comment 14 Adam Klein 2011-04-21 15:20:17 PDT
Patch was rolled out, need to fix the chromium windows build.
Comment 15 Adam Klein 2011-04-27 10:44:38 PDT
Fixing this is not trivial (though probably not hard for a seasoned Windows coder). The build problem was due to some code that depends on pthread_once: I suspect the easiest fix will be to replace that with some Windows equivalent.
Comment 16 Michael Nordman 2011-04-27 11:04:51 PDT
(In reply to comment #15)
> Fixing this is not trivial (though probably not hard for a seasoned Windows coder). The build problem was due to some code that depends on pthread_once: I suspect the easiest fix will be to replace that with some Windows equivalent.

Can you provide a pointer of the file/class/method that is causing the the problem?
Comment 17 Adam Klein 2011-04-27 11:10:31 PDT
wtf/FastMall(In reply to comment #16)
> (In reply to comment #15)
> > Fixing this is not trivial (though probably not hard for a seasoned Windows coder). The build problem was due to some code that depends on pthread_once: I suspect the easiest fix will be to replace that with some Windows equivalent.
> 
> Can you provide a pointer of the file/class/method that is causing the the problem?

Source/JavaScriptCore/wtf/FastMalloc.cpp:110

#if ENABLE(WTF_MULTIPLE_THREADS)
static pthread_key_t isForbiddenKey;
static pthread_once_t isForbiddenKeyOnce = PTHREAD_ONCE_INIT;
static void initializeIsForbiddenKey()
{
  pthread_key_create(&isForbiddenKey, 0);
}
...
Comment 18 Michael Nordman 2011-05-03 17:59:03 PDT
(In reply to comment #17)
> > Can you provide a pointer of the file/class/method that is causing the the problem?
> 
> Source/JavaScriptCore/wtf/FastMalloc.cpp:110
> 
> #if ENABLE(WTF_MULTIPLE_THREADS)
> static pthread_key_t isForbiddenKey;
> static pthread_once_t isForbiddenKeyOnce = PTHREAD_ONCE_INIT;
> static void initializeIsForbiddenKey()
> {
>   pthread_key_create(&isForbiddenKey, 0);
> }
> ...

Geez... that's a scary and hairy looking .cpp file.

There are a handful of places where pthread_once is used on JavaScriptCore. Most of them have OS(DARWIN) gaurds around them and some of them also have a not-so-thread-safe impl if not OS(DARWIN). JSLock in the absence of PTHREADS is scary... but it looks like the only pthreads callsite that matters for V8 users FastMalloc.

The callsite in FastMalloc only matters for DEBUG builds (which is not obvious when looking at the .cpp file). This should be implementable in terms of TlsAlloc(), TlsGetValue(), and TlsSetValue() on windows.


#include <windows.h>

static DWORD isForibiddenTlsIndex = TLS_OUT_OF_INDEXES;

static bool isForbidden()
{
    // By default, fastMalloc is allowed so we don't allocate the
    // tls index unless we're asked to make it forbidden.
    return (isForibiddenTlsIndex != TLS_OUT_OF_INDEXES)
           && (TlsGetValue(isForibiddenTlsIndex) == (LPVOID)1);
}

void fastMallocForbid()
{
    if (isForibiddenTlsIndex == TLS_OUT_OF_INDEXES)
        isForibiddenTlsIndex = TlsAlloc();  // a little racey, but close enough for debug only
    TlsSetValue(isForibiddenTlsIndex, (LPVOID)1);
}

void fastMallocAllow()
{
    if (isForibiddenTlsIndex == TLS_OUT_OF_INDEXES)
        return;
    TlsSetValue(isForibiddenTlsIndex, (LPVOID)0);
}
Comment 19 Adam Klein 2011-05-05 11:42:51 PDT
Thanks for taking a look, Michael; would you be up for uploading that patch as a patch?  If not, I'll do my best to get to it before the M13 branch date (but Windows dev has a good bit of overhead for me so I try to avoid it when possible...)
Comment 20 Michael Nordman 2011-05-05 12:49:42 PDT
(In reply to comment #19)
> Thanks for taking a look, Michael; would you be up for uploading that patch as a patch?  If not, I'll do my best to get to it before the M13 branch date (but Windows dev has a good bit of overhead for me so I try to avoid it when possible...)

Sure... I'll post a patch.
Comment 21 Michael Nordman 2011-05-05 17:54:52 PDT
Created attachment 92515 [details]
fastMalloc windows fix
Comment 22 WebKit Review Bot 2011-05-05 17:57:00 PDT
Attachment 92515 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/FastMalloc.cpp:127:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Michael Nordman 2011-05-05 18:43:25 PDT
Created attachment 92526 [details]
fastMalloc windows fix
Comment 24 Michael Nordman 2011-05-06 17:37:45 PDT
ping

i'm just now looking at the prior patch to enable the flag, i could roll that change into the fastMalloc patch too if you like
Comment 25 Adam Barth 2011-05-06 21:25:28 PDT
Comment on attachment 92526 [details]
fastMalloc windows fix

I wanted to review your patch, but I don't understand this code.  Seems reasonable though.
Comment 26 Jian Li 2011-05-09 12:37:02 PDT
Comment on attachment 92526 [details]
fastMalloc windows fix

View in context: https://bugs.webkit.org/attachment.cgi?id=92526&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        https://bugs.webkit.org/show_bug.cgi?id=55728

Probably it is better to create a new bug for this issue.

> Source/JavaScriptCore/wtf/FastMalloc.cpp:110
> +#if OS(WINDOWS)

It might be better to keep the function defintion unguarded and instead ifdef the implementation.

> Source/JavaScriptCore/wtf/FastMalloc.cpp:121
> +           && (TlsGetValue(isForibiddenTlsIndex) == (LPVOID)1);

No need to separate into 2 lines.
Also better to use static_cast<LPVOID>(1).
Comment 27 Michael Nordman 2011-05-09 12:52:17 PDT
Comment on attachment 92526 [details]
fastMalloc windows fix

Thnx for looking jian!

View in context: https://bugs.webkit.org/attachment.cgi?id=92526&action=review

>> Source/JavaScriptCore/ChangeLog:8

> 
> Probably it is better to create a new bug for this issue.

how about i just merge the two patches into one, define the FLAG (as in adam's patch) and fix the windows bustage (as in my patch)

>> Source/JavaScriptCore/wtf/FastMalloc.cpp:110
>> +#if OS(WINDOWS)
> 
> It might be better to keep the function defintion unguarded and instead ifdef the implementation.

Did you look at how this was done for pthreads in the #else clause and for the cases where WTF_MULTIPLE_THREADS is not defined. Why should anything be different in the OS(WINDOWS) case?

>> Source/JavaScriptCore/wtf/FastMalloc.cpp:121
>> +           && (TlsGetValue(isForibiddenTlsIndex) == (LPVOID)1);
> 
> No need to separate into 2 lines.
> Also better to use static_cast<LPVOID>(1).

ah... of course... the no line is too long rule
Comment 28 Michael Nordman 2011-05-09 13:19:55 PDT
Created attachment 92838 [details]
merged patches

- merged the patches into one
- made longer lines
- use c++ cast operators instead of c-style
Comment 29 David Levin 2011-05-09 13:27:41 PDT
(In reply to comment #28)
> Created an attachment (id=92838) [details]
> merged patches
> 
> - merged the patches into one
> - made longer lines
> - use c++ cast operators instead of c-style

btw, how did this compile for WebKit Window before? Does this break what was happening there previously?
Comment 30 Michael Nordman 2011-05-09 13:38:50 PDT
> btw, how did this compile for WebKit Window before? Does this break what was happening there previously?

Oh... I was under the impression that ENABLE_WTF_MULTIPLE_THREADS isn't turned on for the webkit windows build. If that's not the case than idk?
Comment 31 Adam Roben (:aroben) 2011-05-09 13:45:05 PDT
(In reply to comment #30)
> > btw, how did this compile for WebKit Window before? Does this break what was happening there previously?
> 
> Oh... I was under the impression that ENABLE_WTF_MULTIPLE_THREADS isn't turned on for the webkit windows build. If that's not the case than idk?

ENABLE_WTF_MULTIPLE_THREADS is turned on for Apple's Windows port, as can be seen by looking at Platform.h: <http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/Platform.h?rev=85719#L577>
Comment 32 WebKit Review Bot 2011-05-09 14:13:51 PDT
Comment on attachment 92838 [details]
merged patches

Attachment 92838 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8645642

New failing tests:
http/tests/inspector/console-resource-errors.html
fast/forms/input-stepup-stepdown.html
http/tests/inspector/console-xhr-logging.html
svg/custom/text-repaint-including-stroke.svg
fast/forms/input-valueasnumber-range.html
fast/css/number-parsing-crash-2.html
fast/forms/ValidityState-stepMismatch.html
http/tests/inspector/network/download.html
http/tests/inspector/network-preflight-options.html
fast/forms/input-stepup-stepdown-from-renderer.html
http/tests/inspector/network/ping.html
fast/css/number-parsing-crash.html
fast/forms/ValidityState-rangeOverflow.html
fast/css/round-trip-values.html
http/tests/inspector/change-iframe-src.html
fast/forms/ValidityState-typeMismatch-number.html
fast/css/large-value-csstext.html
fast/forms/input-valueasnumber-number.html
svg/custom/js-update-bounce.svg
fast/forms/ValidityState-rangeUnderflow.html
Comment 33 WebKit Review Bot 2011-05-09 14:13:56 PDT
Created attachment 92848 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 34 Michael Nordman 2011-05-09 14:29:46 PDT
> ENABLE_WTF_MULTIPLE_THREADS is turned on for Apple's Windows port, as can be seen by looking at Platform.h: <http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/Platform.h?rev=85719#L577>

ah... yes, i see that... so where does the win build pick up pthread.h from or is it that fastmalloc.cpp is not used in that build? i'll try to unravel what the deal is with this file and get back to this afterwards.
Comment 35 David Levin 2011-05-11 17:07:14 PDT
Comment on attachment 92838 [details]
merged patches

I don't think this can be landed as is due to the issue mentioned above about WebKit Windows.
Comment 36 Michael Nordman 2011-05-16 20:35:38 PDT
I don't have a webkit win client that builds so it's not so easy to poke at this.

I do see this in WTFCommon.vsprops, $(WebKitLibrariesDir)\include\pthreads
And there is a pthread.h and matching lib file in WebKitLibraries\win
 *      Pthreads-win32 - POSIX Threads Library for Win32
 *      Copyright(C) 1998 John E. Bossom

So i suppose webkit/win is using that lib for some stuff and FastMalloc.cpp complies and links by virtue of it. Afaik, chrome/win, including chromium's webkit port for windows,does not use a library like that. 

This may be a good place for judicious use of PLATFORM(CHROMIUM).
Comment 37 David Levin 2011-05-16 20:59:47 PDT
(In reply to comment #36)
> I don't have a webkit win client that builds so it's not so easy to poke at this.
> 
> I do see this in WTFCommon.vsprops, $(WebKitLibrariesDir)\include\pthreads
> And there is a pthread.h and matching lib file in WebKitLibraries\win
>  *      Pthreads-win32 - POSIX Threads Library for Win32
>  *      Copyright(C) 1998 John E. Bossom
> 
> So i suppose webkit/win is using that lib for some stuff and FastMalloc.cpp complies and links by virtue of it. Afaik, chrome/win, including chromium's webkit port for windows,does not use a library like that. 
> 
> This may be a good place for judicious use of PLATFORM(CHROMIUM).

Sounds good to me.
Comment 38 Michael Nordman 2011-05-17 13:57:49 PDT
> > This may be a good place for judicious use of PLATFORM(CHROMIUM).
> 
> Sounds good to me.

Based on comment #32, looks like setting ENABLE_WTF_MULTIPLE_THREADS does have bad consequences for chromium on linux. A more focused change right now to get rid of the particular ASSERTION we're running into would be to not set that flag, and instead poke directly at CryptographicallyRandomNumber.cpp.

#if ENABLE(WTF_MULTIPLE_THREADS) || PLATFORM(CHROMIUM)
#define THREAD_SAFE_ARC4_RAND 1
#endif

class ARC4RandomNumberGenerator {
public:
    ARC4RandomNumberGenerator();

    uint32_t randomNumber();
    void randomValues(void* buffer, size_t length);

private:
    inline void addRandomData(unsigned char *data, int length);
    void stir();
    void stirIfNeeded();
    inline uint8_t getByte();
    inline uint32_t getWord();

    ARC4Stream m_stream;
    int m_count;
#if THREAD_SAFE_ARC4_RAND
    Mutex m_mutex;
#endif
};
Comment 39 David Levin 2011-05-17 14:16:40 PDT
(In reply to comment #38)
> > > This may be a good place for judicious use of PLATFORM(CHROMIUM).
> > 
> > Sounds good to me.
> 
> Based on comment #32, looks like setting ENABLE_WTF_MULTIPLE_THREADS does have bad consequences for chromium on linux. A more focused change right now to get rid of the particular ASSERTION we're running into would be to not set that flag, and instead poke directly at CryptographicallyRandomNumber.cpp.
> 
> #if ENABLE(WTF_MULTIPLE_THREADS) || PLATFORM(CHROMIUM)
> #define THREAD_SAFE_ARC4_RAND 1
> #endif
> 
> class ARC4RandomNumberGenerator {
> public:
>     ARC4RandomNumberGenerator();
> 
>     uint32_t randomNumber();
>     void randomValues(void* buffer, size_t length);
> 
> private:
>     inline void addRandomData(unsigned char *data, int length);
>     void stir();
>     void stirIfNeeded();
>     inline uint8_t getByte();
>     inline uint32_t getWord();
> 
>     ARC4Stream m_stream;
>     int m_count;
> #if THREAD_SAFE_ARC4_RAND
>     Mutex m_mutex;
> #endif
> };

It feels like there may be a deeper issue that is being glossed over with that failure from #32. If you look at what ENABLE_WTF_MULTIPLE_THREADS does, I wonder why it would cause any problems.
Comment 40 Michael Nordman 2011-05-17 15:56:21 PDT
> It feels like there may be a deeper issue that is being glossed over with that failure from #32. If you look at what ENABLE_WTF_MULTIPLE_THREADS does, I wonder why it would cause any problems.

The failures in #32 indicate that there is something more going on with ENABLE_WTF_MULTIPLE_THREADS, at least in our linux port.  I really wouldn't know anything about that. While we definitely would need to come to terms with that to enable v8 isolates, I don't see where that needs prying into in order to enable random number generation on a background thread.

There actually aren't all that many references to WTF_MULTIPLE_THREAD...

  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\CryptographicallyRandomNumber.cpp(42):#if ENABLE(WTF_MULTIPLE_THREADS) || PLATFORM(CHROMIUM)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\dtoa.cpp(93):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\dtoa.cpp(438):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\dtoa.cpp(453):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\dtoa.cpp(466):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\dtoa.cpp(479):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\FastMalloc.cpp(82):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\FastMalloc.cpp(105):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\FastMalloc.cpp(163):#else // ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\FastMalloc.cpp(180):#endif // ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\Platform.h(582):#define ENABLE_WTF_MULTIPLE_THREADS 1
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\RandomNumber.cpp(64):#if !ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\RefCountedLeakCounter.cpp(82):#if ENABLE(WTF_MULTIPLE_THREADS)
  D:\src\chrome\3\src\third_party\WebKit\Source\JavaScriptCore\wtf\RefCountedLeakCounter.cpp(91):#if ENABLE(WTF_MULTIPLE_THREADS)

Looks like s_dtoaP5Mutex is NULL because it's not initialized here in JavaScriptCore/runtime/InitializeThreading.cpp.
That file is not built into chromium.

static void initializeThreadingOnce()
{
    // StringImpl::empty() does not construct its static string in a threadsafe fashion,
    // so ensure it has been initialized from here.
    StringImpl::empty();

    WTF::initializeThreading();
    wtfThreadData();
    JSGlobalData::storeVPtrs();
#if ENABLE(JSC_MULTIPLE_THREADS)
    s_dtoaP5Mutex = new Mutex;
    initializeDates();
    RegisterFile::initializeThreading();
#endif
}
Comment 41 David Levin 2011-05-17 16:09:09 PDT
(In reply to comment #40)
> 
> Looks like s_dtoaP5Mutex is NULL because it's not initialized here in JavaScriptCore/runtime/InitializeThreading.cpp.
> That file is not built into chromium.
> 
> static void initializeThreadingOnce()
> {
>     // StringImpl::empty() does not construct its static string in a threadsafe fashion,
>     // so ensure it has been initialized from here.
>     StringImpl::empty();
> 
>     WTF::initializeThreading();
>     wtfThreadData();
>     JSGlobalData::storeVPtrs();
> #if ENABLE(JSC_MULTIPLE_THREADS)
>     s_dtoaP5Mutex = new Mutex;
>     initializeDates();
>     RegisterFile::initializeThreading();
> #endif
> }


Good point. 

Both 
    s_dtoaP5Mutex = new Mutex;
    initializeDates();
are in the wrong place.

Seems like 
     // StringImpl::empty() does not construct its static string in a threadsafe fashion,
     // so ensure it has been initialized from here.
     StringImpl::empty();
     wtfThreadData();
Should also be done in wtf.

btw, the asserts are about to be removed from this method: https://bugs.webkit.org/show_bug.cgi?id=60846 so this issue won't be noticed.

The real issue here (imo) is getting WTF_MULTIPLE_THREADS turned on for chromium but the current fire is won't be there shortly.
Comment 42 Michael Nordman 2011-05-17 16:12:56 PDT
> Good point. 
> 
> Both 
>     s_dtoaP5Mutex = new Mutex;
>     initializeDates();
> are in the wrong place.
> 
> Seems like 
>      // StringImpl::empty() does not construct its static string in a threadsafe fashion,
>      // so ensure it has been initialized from here.
>      StringImpl::empty();
>      wtfThreadData();
> Should also be done in wtf.
> 
> btw, the asserts are about to be removed from this method: https://bugs.webkit.org/show_bug.cgi?id=60846 so this issue won't be noticed.
> 
> The real issue here (imo) is getting WTF_MULTIPLE_THREADS turned on for chromium but the current fire is won't be there shortly.

Ah... i like abarth's lean on the delete key solution even more than the narrow #if PLATFORM(CHROMIUM) :)
Comment 43 Michael Nordman 2011-05-17 17:31:21 PDT
> Both 
>     s_dtoaP5Mutex = new Mutex;
>     initializeDates();
> are in the wrong place.
> 
> Seems like 
>      // StringImpl::empty() does not construct its static string in a threadsafe fashion,
>      // so ensure it has been initialized from here.
>      StringImpl::empty();
>      wtfThreadData();
> Should also be done in wtf.

All that looks out of place to me too, needs to be initted for WTF independently of JSC. Is there a WTF::initialize() call anywhere, that would be a good place to put get the TLS slot defined for FastMalloc on windows (the non-pthreads version) too.

> btw, the asserts are about to be removed from this method: https://bugs.webkit.org/show_bug.cgi?id=60846 so this issue won't be noticed.
> 
> The real issue here (imo) is getting WTF_MULTIPLE_THREADS turned on for chromium but the current fire is won't be there shortly.

That wasn't the issue i came here to fix, but yes it certainly is the bigger fish to fry. Do we have a timeframe for putting v8 isolates to use? Is that actively being worked on at this point?
Comment 44 David Levin 2011-05-17 19:10:29 PDT
(In reply to comment #43)
> That wasn't the issue i came here to fix, but yes it certainly is the bigger fish to fry. Do we have a timeframe for putting v8 isolates to use? Is that actively being worked on at this point?

Yep, bug 61016. I also filed bug 61017 about turning on WTF_MULTIPLE_THREADS for chromium.

I don't guess you need to do any more here (once bug 60846 lands).
Comment 45 Michael Nordman 2011-05-26 11:07:12 PDT
should be fixed with 60846 being fixed