Refactor isolate interrupts to use OOB messages instead of interrupt bits.

Refactor isolate interrupts to use OOB messages instead of interrupt bits.

Closes #24349

R=johnmccutchan@google.com

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

[turnidge, 2015-09-15 22:14:09]

turnidge@google.com changed reviewers:
+ iposva@google.com

[turnidge, 2015-09-15 22:14:10]

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

https://codereview.chromium.org/1344993002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:356: // If any error occurred while in the debug message
loop, return it here.
This could happen, for example, if the isolate receives a kill message while in
the debugger message loop.

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

https://codereview.chromium.org/1344993002/diff/1/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:624: if (result.IsUnwindError() || I->ErrorsFatal()) {
This isn't quite right, probably, but I wanted to get the code out for review. 
We probably want to ignore listeners for unwind errors.

[turnidge, 2015-09-16 19:20:04]

turnidge@google.com changed reviewers:
+ zra@google.com

[turnidge, 2015-09-16 19:20:05]

Adding Zach...

[zra, 2015-09-16 20:19:16]

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/debugger.dart (right):

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/debugger.dart:1361: "Paused at isolate
start (type 'continue' [F7] or 'step' [F10] to start the isolate')");
Since you're in the neighborhood, you might fix these long lines.

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/pause_idle_isolate_test.dart (right):

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/pause_idle_isolate_test.dart:57: // Wait for
the isolate to become idle.  We detect this by querying
Unrelated to this CL, but this feels like a hole in the API.

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

https://codereview.chromium.org/1344993002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:484: success = ProcessUnhandledException(error);
ProcessUnhandledException invokes Isolate::UnhandledExceptionCallback() even
when its argument is not an UnhandledException. `error` here could be an
UnwindError coming from an OOB Kill message. I don't think we want to invoke the
callback in that case.

https://codereview.chromium.org/1344993002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:511: success = ProcessUnhandledException(error);
ditto

https://codereview.chromium.org/1344993002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:593: if (Isolate::UnhandledExceptionCallback() != NULL) {
UnhandledExceptionCallback is here.

[turnidge, 2015-09-22 17:37:38]

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/debugger.dart (right):

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/debugger.dart:1361: "Paused at isolate
start (type 'continue' [F7] or 'step' [F10] to start the isolate')");
On 2015/09/16 20:19:16, zra wrote:
> Since you're in the neighborhood, you might fix these long lines.

Done.

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/pause_idle_isolate_test.dart (right):

https://codereview.chromium.org/1344993002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/pause_idle_isolate_test.dart:57: // Wait for
the isolate to become idle.  We detect this by querying
On 2015/09/16 20:19:16, zra wrote:
> Unrelated to this CL, but this feels like a hole in the API.

Perhaps a protocol user would want to be notified when an isolate becomes idle,
yes.  Will consider.

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

https://codereview.chromium.org/1344993002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:484: success = ProcessUnhandledException(error);
On 2015/09/16 20:19:16, zra wrote:
> ProcessUnhandledException invokes Isolate::UnhandledExceptionCallback() even
> when its argument is not an UnhandledException. `error` here could be an
> UnwindError coming from an OOB Kill message. I don't think we want to invoke
the
> callback in that case.

This is an embedder provided callback -- what if it expects to be called to
deallocate resources?  Putting in a TODO.

[turnidge, 2015-09-22 17:55:53]

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

[turnidge, 2015-09-22 17:55:53]

Changing reviewer to Cutch.

[Cutch, 2015-09-22 18:21:01]

LGTM

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

https://codereview.chromium.org/1344993002/diff/60001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:346: if (callback) {
nit: callback != NULL

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

https://codereview.chromium.org/1344993002/diff/60001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1474: message_handler()->set_pause_on_exit(false);
If an isolate is terminated via an OOB message- do we want the service's pause
on exit to keep it around?

[turnidge, 2015-09-23 18:18:44]

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

https://codereview.chromium.org/1344993002/diff/60001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:346: if (callback) {
On 2015/09/22 18:21:01, Cutch wrote:
> nit: callback != NULL

Done.

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

https://codereview.chromium.org/1344993002/diff/60001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1474: message_handler()->set_pause_on_exit(false);
On 2015/09/22 18:21:01, Cutch wrote:
> If an isolate is terminated via an OOB message- do we want the service's pause
> on exit to keep it around?

Added a TODO

[turnidge, 2015-09-23 19:13:29]

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