Call shutdown callback before the isolate is destroyed

Call shutdown callback before the isolate is destroyed

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24844

[siva, 2013-07-08 21:54:39]

One question regarding accessing the delete isolate pointer in
StartIsolateScope::~StartIsolateScope

https://codereview.chromium.org/18862004/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/18862004/diff/1/runtime/include/dart_api.h#ne...
runtime/include/dart_api.h:671: * isolate. It is safe run Dart code.
safe to

https://codereview.chromium.org/18862004/diff/1/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/18862004/diff/1/runtime/vm/isolate.cc#newcode652
runtime/vm/isolate.cc:652: // Shut the isolate down.
I think this may end up accessing the deleted isolate pointer in
~StartIsolateScope if this function is called by an Isolate other than the one
being shutdown.

Did you run into an issue with this?

[siva, 2013-07-08 22:39:27]

lgtm

https://codereview.chromium.org/18862004/diff/8001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/18862004/diff/8001/runtime/include/dart_api.h...
runtime/include/dart_api.h:671: * isolate. It is safe run Dart code.
"safe to"

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:802: Isolate* isolate = Isolate::Current();
Move this line up and change:
CHECK_ISOLATE(isolate);

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:804: StartIsolateScope start_scope(isolate);
StartIsolateScope is pretty much a NOP here as
saved_isolate_ is always equal to new_isolate_

[Cutch, 2013-07-08 22:43:24]

https://codereview.chromium.org/18862004/diff/8001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/18862004/diff/8001/runtime/include/dart_api.h...
runtime/include/dart_api.h:671: * isolate. It is safe run Dart code.
On 2013/07/08 22:39:27, siva wrote:
> "safe to"

Done.

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:802: Isolate* isolate = Isolate::Current();
On 2013/07/08 22:39:27, siva wrote:
> Move this line up and change:
> CHECK_ISOLATE(isolate);

Done.

https://codereview.chromium.org/18862004/diff/8001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:804: StartIsolateScope start_scope(isolate);
On 2013/07/08 22:39:27, siva wrote:
> StartIsolateScope is pretty much a NOP here as
> saved_isolate_ is always equal to new_isolate_

Done.

[Cutch, 2013-07-09 14:59:08]

Committed patchset #5 manually as r24844 (presubmit successful).