Fix issue 906

Fix issue 906

R=iposva@google.com

BUG=dart:906
TEST=

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

[Søren Gjesse, 2011-12-19 16:19:53]

I am still wondering why the OS::Sleep(5) is still required for the test to not
deadlock.

[Ivan Posva, 2011-12-19 16:27:46]

LGTM -ip

http://codereview.chromium.org/8995008/diff/1/runtime/vm/dart_api_impl_test.cc
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8995008/diff/1/runtime/vm/dart_api_impl_test.c...
runtime/vm/dart_api_impl_test.cc:2898: // Tell the other thread that we are
done.
This block should move after the isolate has been shutdown. Then I think we can
avoid the OS:Sleep(20) in the driver.

[siva, 2011-12-19 22:02:12]

Søren,

The deadlock you were observing without the OS::Sleep(5) is because in this case
three methods are run before 'main' is invoked and we exhaust all the interrupts
in these methods even before 'main' is invoked. We end up processing all the
interrupts and then invoke 'main' which goes into the endless loop without any
interrupts.

> Entering 'StackOverflowException.StackOverflowException.'
 =========== Interrupt callback called #1
> Entering 'IntegerImplementation.&'
 =========== Interrupt callback called #2
> Entering 'IntegerImplementation.bitAndFromInteger'
 =========== Interrupt callback called #3
 =========== Interrupt callback Throwing exception
handler_pc = 0xf0b0006d
> Entering '::.main'
> Entering '::.function'
< Exiting '::.function'
> Entering '::.function'
< Exiting '::.function'
> Entering '::.function'
< Exiting '::.function'

[Søren Gjesse, 2011-12-20 09:21:26]

Siva, thanks for figuring out that main of cause have to be entered before
starting to issue interrupts.

I have updated the test (Patch Set 2) to synchronize on entering main. I also
moved the synchronization on the isolate exit. I thereby got rid of all
OS::Sleep calls.

I am committing it now to get rid of the Windows issue, but please review once
more.

http://codereview.chromium.org/8995008/diff/1/runtime/vm/dart_api_impl_test.cc
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8995008/diff/1/runtime/vm/dart_api_impl_test.c...
runtime/vm/dart_api_impl_test.cc:2898: // Tell the other thread that we are
done.
On 2011/12/19 16:27:50, Ivan Posva wrote:
> This block should move after the isolate has been shutdown. Then I think we
can
> avoid the OS:Sleep(20) in the driver.

Done.

[Ivan Posva, 2011-12-21 08:31:16]

http://codereview.chromium.org/8995008/diff/5001/runtime/vm/dart_api_impl_tes...
File runtime/vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8995008/diff/5001/runtime/vm/dart_api_impl_tes...
runtime/vm/dart_api_impl_test.cc:3091: for (int i = 0; i < 3; i++) {
I would prefer if we spaced the interrupts out a bit.

For example:
OS::Sleep(i * 1);

While at it can you also get rid of the magic value 3 by using a constant?