Fixes crashes in VM isolate shutdown.

Fixes crashes in VM isolate shutdown.

If null is not initialized before the VM Isolate's ObjectStore, the
ObjectStore's fields are initialized to kHeapObjectTag, which causes
a crash in VM Isolate shutdown.

There is also a crash when the VM Isolate is signaled with the
shutdown event, so this CL removes adding a Debugger to the
VM Isolate, so that there is no need to shut it down, and send
the signal.

NB: This change does *not* actually enable shutdown. It just fixes
crashes that would happen if it were enabled.

BUG=
R=iposva@google.com, johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/fcd4c59430840348d08bd8ad21b46de21347dc2b

[zra, 2015-06-09 14:52:16]

zra@google.com changed reviewers:
+ iposva@google.com, johnmccutchan@google.com

[zra, 2015-06-09 14:52:16]

These crashes are triggered when VM Isolate shutdown is re-enabled in
Dart::Cleanup() in runtime/vm/dart.cc.

[Cutch, 2015-06-09 14:57:33]

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

https://codereview.chromium.org/1168933002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1219: (isolate_ != Dart::vm_isolate())) {
I think the real bug is that the vm-isolate shouldn't have a debugger in the
first place.

[zra, 2015-06-09 15:45:18]

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

https://codereview.chromium.org/1168933002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1219: (isolate_ != Dart::vm_isolate())) {
On 2015/06/09 14:57:33, Cutch wrote:
> I think the real bug is that the vm-isolate shouldn't have a debugger in the
> first place.

Ah, yah. That makes sense. I'll see if I can get rid of it.

[zra, 2015-06-09 17:29:02]

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

https://codereview.chromium.org/1168933002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1219: (isolate_ != Dart::vm_isolate())) {
On 2015/06/09 15:45:17, zra wrote:
> On 2015/06/09 14:57:33, Cutch wrote:
> > I think the real bug is that the vm-isolate shouldn't have a debugger in the
> > first place.
> 
> Ah, yah. That makes sense. I'll see if I can get rid of it.

Done. Removed debugger from vm isolate, and added asserts on access to
isolate->debugger().

[Cutch, 2015-06-09 17:33:47]

lgtm

[zra, 2015-06-16 15:42:06]

@iposva: ping

[Ivan Posva, 2015-06-16 16:31:15]

Wondering where we are accessing the RAW_NULL.

More comments below.

-Ivan

https://codereview.chromium.org/1168933002/diff/40001/runtime/vm/debugger_api...
File runtime/vm/debugger_api_impl.cc (right):

https://codereview.chromium.org/1168933002/diff/40001/runtime/vm/debugger_api...
runtime/vm/debugger_api_impl.cc:366: ASSERT(debugger != NULL);
Where is the difference to this line?

ditto at other places...

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

https://codereview.chromium.org/1168933002/diff/40001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1535: ASSERT(debugger() != NULL);
if (debugger() != NULL) {
  ...
}

Since we are visiting the VM isolate in certain situations.

https://codereview.chromium.org/1168933002/diff/40001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1592: ASSERT(debugger() != NULL);
We should allow printing of the VM isolate.

[zra, 2015-06-16 21:15:39]

Updated as discussed. PTAL.

[Ivan Posva, 2015-06-16 23:55:52]

LGTM

You could remove all of the sprinkled ASSERT(isolate->debugger() != NULL) across
the code. Not sure if any are left and replace it with an assertion inside the
debugger() accessor in Isolate.

-Ivan

[zra, 2015-06-17 17:07:33]

On 2015/06/16 23:55:52, Ivan Posva wrote:
> You could remove all of the sprinkled ASSERT(isolate->debugger() != NULL)
across
> the code. Not sure if any are left and replace it with an assertion inside the
> debugger() accessor in Isolate.

Done.

[zra, 2015-06-17 17:14:23]

Committed patchset #5 (id:80001) manually as
fcd4c59430840348d08bd8ad21b46de21347dc2b (presubmit successful).