Sending the code review to you I should have guessed. Changed all Handle initializations to assignments in the function (previously 2 where assignments and 2 where not).
On Mon, Oct 6, 2008 at 3:01 PM, Kasper Lund <[EMAIL PROTECTED]> wrote: > LGTM. I would use assignment for the initialization of 'wrapper', but > that's probably just me. > > On Mon, Oct 6, 2008 at 2:58 PM, <[EMAIL PROTECTED]> wrote: > > Reviewers: Kasper Lund, > > > > Description: > > Fixed unsafe code where a GC could occour after a Handle had been > > deferenced. > > > > instances->set(i, *GetScriptWrapper(script)); > > > > GetScriptWrapper can call GC. The failure have only been seen on ARM, > > where > > the g++ compiler pulls out the object from the instances handle to a > > register > > before calling GetScriptWrapper causing set to be called on an object > > which > > may have moved. > > > > Marked a test on ARM as no longer flaky, whereas two other fails > > consistently > > but that is no longer related to the problem fixed above. > > > > BUG=1308895 > > > > Please review this at http://codereview.chromium.org/6271 > > > > Affected files: > > M src/runtime.cc > > M test/mjsunit/mjsunit.status > > > > > > Index: test/mjsunit/mjsunit.status > > =================================================================== > > --- test/mjsunit/mjsunit.status (revision 443) > > +++ test/mjsunit/mjsunit.status (working copy) > > @@ -57,11 +57,7 @@ > > debug-step-stub-callfunction: FAIL > > debug-stepin-constructor: FAIL > > debug-step: FAIL > > -regress/regress-998565: FAIL > > - > > -# Bug number 1308895: These tests pass on the ARM simulator, but > > -# fail on the ARM Linux machine. > > -debug-script-breakpoints: PASS || FAIL > > -debug-scripts-request: PASS || FAIL > > +debug-script-breakpoints: FAIL > > debug-breakpoints: PASS || FAIL > > > > +regress/regress-998565: FAIL > > Index: src/runtime.cc > > =================================================================== > > --- src/runtime.cc (revision 443) > > +++ src/runtime.cc (working copy) > > @@ -4698,7 +4698,8 @@ > > // Convert the script objects to proper JS objects. > > for (int i = 0; i < count; i++) { > > Handle<Script> script(Script::cast(instances->get(i))); > > - instances->set(i, *GetScriptWrapper(script)); > > + Handle<JSValue> wrapper(GetScriptWrapper(script)); > > + instances->set(i, *wrapper); > > } > > > > // Return result as a JS array. > > > > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
