Thanks for the comments!

  Luke


http://codereview.chromium.org/2910004/diff/1/11
File src/isolate.cc (right):

http://codereview.chromium.org/2910004/diff/1/11#newcode197
src/isolate.cc:197: static bool static_initialized =
Isolate::EnsureStaticInitialized();
On 2010/07/09 21:45:55, Vitaly wrote:
Why do you need this bool?

I need to call EnsureStaticInitialized() myself in the (very common)
case that
the client of the library does not call it for me.

http://codereview.chromium.org/2910004/diff/1/11#newcode254
src/isolate.cc:254: USE(static_initialized);
On 2010/07/09 21:45:55, Vitaly wrote:
What's the expected effect of this?

At the moment I just want to avoid the warning regarding the symbol
being unused. Is there a better trick?

http://codereview.chromium.org/2910004/diff/1/11#newcode257
src/isolate.cc:257: default_isolate_ = new_default_isolate;
On 2010/07/09 21:45:55, Vitaly wrote:
I'm afraid this doesn't make the double-checked-locking here safe. You
need a
memory barrier before assigning to "default_isolate_". Try using
ReleaseStore.
(See http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf) Or
better yet
rewrite this to always grab the mutex -- I don't think initialization
is that
performance critical.

Thanks for the tip-- I had some vague uneasiness about memory issues
here.

http://codereview.chromium.org/2910004/diff/1/11#newcode263
src/isolate.cc:263: // TODO(isolates): Check that we're not in
multiple-isolate mode.
On 2010/07/09 21:34:37, Dmitry Titov wrote:
Could you please clarify what should be checked here? If other thread
creates an
isolate a millisecond later, whats' going to be wrong?

To this and the previous comment: IIRC it is invalid, once we enter the
multiple-isolate state, to implicitly enter the default isolate. Hence
once we
have the machinery for multiple isolates in place, TryToImplicitly...
can fail a
logic CHECK().

http://codereview.chromium.org/2910004/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to