OK, I learned the implementation of threading in V8 and updated my code.
I also wrote a test to be confident.

What I changed in v8threads is that thread ids are now 1-based, instead
of 0-based. As NULL is defined as 0, having a thread id value 0 in TLS
is indistinguishable from not having this value stored.

Mads, Soeren, can you take another look, please?


http://codereview.chromium.org/171115/diff/3001/4001
File src/platform-linux.cc (right):

http://codereview.chromium.org/171115/diff/3001/4001#newcode619
Line 619: // it means that the VM thread is running, and trying to
sample it will
On 2009/09/03 12:08:53, Mads Ager wrote:
> Could you update the comment to state that only the stack traversal is
unsafe in
> this case?  The rest of the sampling takes place as normal.

Done.

http://codereview.chromium.org/171115/diff/3001/4001#newcode626
Line 626: || ThreadManager::CurrentId() != 0) return true;
On 2009/09/03 12:08:53, Mads Ager wrote:
> Could you move the '||' to the previous line to match (most) of the
rest of the
> code?

> I think the first thread will get thread_id 0, so this check will not
be
> precise.  We should probably add a ThreadManager::HasThreadId method
instead
> that returns Thread::HasThreadLocal(thread_id_key).

> Thinking about this some more, there is no guarantee that the current
thread is
> the thread using V8 just because it has a v8 thread id.  It could be
doing stuff
> that does not involve v8 when it gets the interrupt.

Done.

http://codereview.chromium.org/171115

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

Reply via email to