[Webkit-unassigned] [Bug 27655] [v8] cache v8 strings when converting from webcore string to v8 string

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33619|review?                     |review-
               Flag|                            |




--- Comment #8 from David Levin <levin at chromium.org>  2009-07-28 08:48:17 PDT ---
(From update of attachment 33619)
"
> +#include "StringHash.h"
> +#include "Threading.h"
> +#include "ThreadSpecific.h"

Case sensitive sorting, so it should be:

#include "ThreadSpecific.h"
#include "Threading.h"


> +// Returns thread-specific version of string cache
> +static StringCache& getStringCache()
> +{
> +    if (WTF::isMainThread())
> +    {
> +        DEFINE_STATIC_LOCAL(StringCache, mainThreadStringCache, ());
> +        return mainThreadStringCache;
> +    }
> +
> +    DEFINE_STATIC_LOCAL(WTF::ThreadSpecific<StringCache>, threadStringCache, ());


This looks like it has a race condition. (specifically the allocation of
ThreadSpecific<StringCache>.)
Please add a comment about why it isn't or consider switching to using
AtomicallyInitializedStatic (from
wtf/Threading.h).

> +static void cachedStringCallback(v8::Persistent<v8::Value> wrapper, void* parameter)
> +{
> +    StringImpl* stringImpl = static_cast<StringImpl*>(parameter);
> +    ASSERT(getStringCache().contains(stringImpl));
> +    getStringCache().remove(stringImpl);
> +    wrapper.Dispose();
> +    stringImpl->deref();

It is my understanding that v8 garbage collection may be done on any thread. 
If that is the case, then the code would be broken because getStringCache()
returns a thread specific value.

(Even if you got the right string cache, you'd run into problems with
StringCache and StringImpl::Deref not being threadsafe.)

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