Add support for interrupting an isolate in the vm. Interrupts are

Add support for interrupting an isolate in the vm.  Interrupts are
implemented by extending the existing support for stack overflow
checking in the vm.  When an interrupt is scheduled for an isolate, we
overwrite the isolate's stack limit with a value guaranteed to cause a
stack overflow.

We support multiple kinds of interrupts, encoded in the low bits of
the stack limit.

Add Dart_InterruptIsolate and Dart_InterruptIsolateCallback to the
dart embedding api to allow the embedder to request and handle
interrupts.

Add EXPECT_SUBSTRING(needle, haystack) testing macro.
Committed: https://code.google.com/p/dart/source/detail?r=2529

[turnidge, 2011-12-07 19:38:14]

There is still a TODO in code_generator_ia32.cc about a bunch of nops I added.

[srdjan, 2011-12-12 22:12:50]

DBC

http://codereview.chromium.org/8851008/diff/21002/runtime/vm/assert.h
File runtime/vm/assert.h (right):

http://codereview.chromium.org/8851008/diff/21002/runtime/vm/assert.h#newcode52
runtime/vm/assert.h:52: void Substring(const E& needle, const A& haystack);
IsSubstring?

[turnidge, 2011-12-12 22:20:32]

http://codereview.chromium.org/8851008/diff/21002/runtime/vm/assert.h
File runtime/vm/assert.h (right):

http://codereview.chromium.org/8851008/diff/21002/runtime/vm/assert.h#newcode52
runtime/vm/assert.h:52: void Substring(const E& needle, const A& haystack);
On 2011/12/12 22:12:50, srdjan wrote:
> IsSubstring?

Done.

[Ivan Posva, 2011-12-14 23:59:50]

First set of comments.

-Ivan

http://codereview.chromium.org/8851008/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/include/dart_api.h#...
runtime/include/dart_api.h:260: typedef bool (*Dart_IsolateInterruptCallback)();
Should we also pass the current isolate as a parameter to avoid the obvious call
to Dart_CurrentIsolate?

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.c...
runtime/vm/code_generator.cc:825: stack_pos = (uword) &stack_pos;
Wrong style cast. How about:
uword stack_pos = reinterpret_cast<uword>&arguments;

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.c...
runtime/vm/code_generator.cc:830: // Use a preallocated stack overflow exception
to avoid calling
"Use the preallocated"?

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator_i...
File runtime/vm/code_generator_ia32.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator_i...
runtime/vm/code_generator_ia32.cc:286: // TODO(turnidge): It would be nice to
remove this nops Right now
"these nops. Right now"

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2781: "void throwException() {\n"
This does not really throw the exception, how about naming it allocException?

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2848: Dart_Handle result =
Dart_ThrowException(exc);
You could also throw a string here. That way you do not have to go through the
dance of allocating the exception.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2867: MonitorLocker ml(sync);
This locking here is strange.

[Ivan Posva, 2011-12-15 19:12:19]

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:159: stack_limit_ = limit;
This will blow away pending interrupts.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:171: stack_limit_ = ((uword) -1) & ~kInterruptsMask;
Old-style cast.

[turnidge, 2011-12-15 21:30:06]

Hi Ivan -- Please take another look.

http://codereview.chromium.org/8851008/diff/24001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/include/dart_api.h#...
runtime/include/dart_api.h:260: typedef bool (*Dart_IsolateInterruptCallback)();
Discussed offline and decided to keep as-is.

On 2011/12/14 23:59:50, Ivan Posva wrote:
> Should we also pass the current isolate as a parameter to avoid the obvious
call
> to Dart_CurrentIsolate?

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.c...
runtime/vm/code_generator.cc:825: stack_pos = (uword) &stack_pos;
On 2011/12/14 23:59:50, Ivan Posva wrote:
> Wrong style cast. How about:
> uword stack_pos = reinterpret_cast<uword>&arguments;

Done.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator.c...
runtime/vm/code_generator.cc:830: // Use a preallocated stack overflow exception
to avoid calling
On 2011/12/14 23:59:50, Ivan Posva wrote:
> "Use the preallocated"?

Done.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator_i...
File runtime/vm/code_generator_ia32.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/code_generator_i...
runtime/vm/code_generator_ia32.cc:286: // TODO(turnidge): It would be nice to
remove this nops Right now
Changed to "this nop", now that it is a single instruction.

On 2011/12/14 23:59:50, Ivan Posva wrote:
> "these nops. Right now"

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2781: "void throwException() {\n"
On 2011/12/14 23:59:50, Ivan Posva wrote:
> This does not really throw the exception, how about naming it allocException?

Removed.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2848: Dart_Handle result =
Dart_ThrowException(exc);
On 2011/12/14 23:59:50, Ivan Posva wrote:
> You could also throw a string here. That way you do not have to go through the
> dance of allocating the exception.

Done.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2867: MonitorLocker ml(sync);
On 2011/12/14 23:59:50, Ivan Posva wrote:
> This locking here is strange.

Fixed.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:159: stack_limit_ = limit;
Fixed.

On 2011/12/15 19:12:20, Ivan Posva wrote:
> This will blow away pending interrupts.

http://codereview.chromium.org/8851008/diff/24001/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:171: stack_limit_ = ((uword) -1) & ~kInterruptsMask;
On 2011/12/15 19:12:20, Ivan Posva wrote:
> Old-style cast.

Done.

[siva, 2011-12-17 00:05:38]

DBC

http://codereview.chromium.org/8851008/diff/41001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/8851008/diff/41001/runtime/include/dart_api.h#...
runtime/include/dart_api.h:416: * Dart_IsolateInterruptCallback).
If Dart_ShutdownIsolate is called by an isolate that has a pending interrupt the
isolate will still be shutdown, Maybe we should make that explicit here.

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/dart_api_impl_te...
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2936: sync = new Monitor();
Where is 'sync' deleted?
Should be done at the end of this function.

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2954: OS::Sleep(5);
These sleeps make the test a little non deterministic right, because the other
thread might not be scheduled and we might end up calling Dart_InterruptIsolate
even before the previous interrupt was serviced.

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/dart_api_impl_te...
runtime/vm/dart_api_impl_test.cc:2969: OS::Sleep(20);
Why is this sleep necessary, won't the other thread just exit independently?

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

http://codereview.chromium.org/8851008/diff/41001/runtime/vm/isolate.cc#newco...
runtime/vm/isolate.cc:158: void Isolate::SetStackLimit(uword limit) {
ASSERT(limit != (~static_cast<uword>(0) & ~kInterruptsMask));