At the moment I cannot find any threading related issues with the code anymore. Some small comments below, otherwise LGTM.
-Ivan http://codereview.chromium.org/39124/diff/1015/30 File src/debug.h (right): http://codereview.chromium.org/39124/diff/1015/30#newcode574 Line 574: // If a preemption is pending clear it and store this information. Comment does not match with the code. Why do you need to test if prev_ is NULL? You are not recording the pending preemption in this case. Did you miss to uncomment your setting of the preemption bit after testing? Otherwise if you do not intend to set it, then you should remove the commented code. http://codereview.chromium.org/39124/diff/1015/30#newcode607 Line 607: if (prev_ == NULL && Debug::preemption_pending()) { Please add a comment why you test for prev_ being NULL. For example: // Request preemption when leaving the last debugger entry and a preemption had been recorded while debugging. http://codereview.chromium.org/39124/diff/1015/32 File src/execution.cc (right): http://codereview.chromium.org/39124/diff/1015/32#newcode527 Line 527: if (!Debug::InDebugger()) { I would probably turn this around to not get the NOT in the test for readability and then the comment also is closer to the described action. http://codereview.chromium.org/39124/diff/1015/33 File test/cctest/test-debug.cc (right): http://codereview.chromium.org/39124/diff/1015/33#newcode53 Line 53: using ::v8::internal::DeleteArray; Why did you have to add these? http://codereview.chromium.org/39124 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
