LGTM. I think one single Locker object can solve all 3 locking issues.
http://codereview.chromium.org/18404/diff/1/2 File src/d8.cc (right): http://codereview.chromium.org/18404/diff/1/2#newcode449 Line 449: while (ptr) { Test against NULL should be explicit. http://codereview.chromium.org/18404/diff/1/2#newcode453 Line 453: if (next) { Test against NULL should be explicit. http://codereview.chromium.org/18404/diff/1/2#newcode457 Line 457: ptr = next; ptr = NULL would be clearer. http://codereview.chromium.org/18404/diff/1/2#newcode475 Line 475: Context::Scope context_scope(evaluation_context_); This should be protected by a locker. Also the place where it goes out of scope - is there a nontrivial destructor? http://codereview.chromium.org/18404/diff/1/2#newcode496 Line 496: Locker::StartPreemption(10); You only need to do this once, not once per file. http://codereview.chromium.org/18404/diff/1/2#newcode505 Line 505: // Use all other arguments as names of files to load and run. If the user uses both -p files and straight js files on the command line we need a lock here. http://codereview.chromium.org/18404/diff/1/2#newcode520 Line 520: RunShell(); and here. http://codereview.chromium.org/18404 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
