Add Isolate.onExit.

Add Isolate.onExit.

Implemented on dart2js.
Tentative design.
Comments welcome.

R=floitsch@google.com

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

[floitsch, 2014-02-25 19:25:48]

FYI (haven't yet finished thinking about it).

Eventually we will probably want to be able to kill a web-worker
(worker.terminate) from the outside. This means that we can't always rely on the
web-worker to cooperate (i.e. send the shutdown events).

https://codereview.chromium.org/179823002/diff/1/sdk/lib/_internal/lib/isolat...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/179823002/diff/1/sdk/lib/_internal/lib/isolat...
sdk/lib/_internal/lib/isolate_helper.dart:313: // If necessary, we can switch
doneHandlers to a Set if it gets larget.
larger.

https://codereview.chromium.org/179823002/diff/1/sdk/lib/_internal/lib/isolat...
sdk/lib/_internal/lib/isolate_helper.dart:391: _globalState.isolates.remove(id);
// indicate this isolate is not active
"I"ndicate ... active"."

https://codereview.chromium.org/179823002/diff/1/sdk/lib/_internal/lib/isolat...
sdk/lib/_internal/lib/isolate_helper.dart:551: * The src url for the script tag
that loaded this Used to create
missing ".".

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart...
sdk/lib/isolate/isolate.dart:47: * the isolate.
I don't think inspecting is the same as being able to see uncaught errors and
exits of isolates.
I have definitely less problems letting someone else know, that I died, than
allowing him to extract all my cookie-information.

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart...
sdk/lib/isolate/isolate.dart:152: // Should we return a broadcast stream
instead? It has the advantage
I don't see any stream.

[Lasse Reichstein Nielsen, 2014-02-27 08:04:27]

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart...
sdk/lib/isolate/isolate.dart:47: * the isolate.
Would "observe" be a better word. It suggests a more hands-off view of only
external behavior, where "inspect" is more like peeking into the internals?

https://codereview.chromium.org/179823002/diff/1/sdk/lib/isolate/isolate.dart...
sdk/lib/isolate/isolate.dart:152: // Should we return a broadcast stream
instead? It has the advantage
No, I was suggesting a broadcast stream instead of a future.
The stream would only ever send a "done" message, so it seems like overdoing it,
but on the other hand, it allows listening, and unlistening again, which a
future doesn't.

[Lasse Reichstein Nielsen, 2014-03-03 11:43:31]

PTAL

[Søren Gjesse, 2014-03-04 10:16:25]

DBC

Please change the subject of the change to match the actual implementation
better.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
sdk/lib/_internal/lib/isolate_helper.dart:307: void addDoneListener(SendPort
response) {
Use name responsePort as in isolate.dart.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
sdk/lib/_internal/lib/isolate_helper.dart:317: void removeDoneListener(SendPort
response) {
Ditto.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:90: /**
I think we should move the WARNING up as the first line in the comment, and
extend it to also say that this is still experimental.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:155: * send a reply if the receiving isolate is
dead?
Add the warning as for pause/resume.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:157: void addOnExitListener(SendPort responsePort)
{
No capability needed to "spy" on when an isolate exits?

[floitsch, 2014-03-04 12:00:16]

LGTM.
Willing to discuss the need for a capability for onExit, but I don't think it's
necessary.

[Lasse Reichstein Nielsen, 2014-03-04 12:12:58]

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

[Søren Gjesse, 2014-03-04 13:44:33]

On 2014/03/04 12:00:16, floitsch wrote:
> LGTM.
> Willing to discuss the need for a capability for onExit, but I don't think
it's
> necessary.

Discussed the capability for onExit with Lasse, and the conclusion was that the
Isolate object (actually the control port) is the base capability needed for
onExit and ping.

[Lasse Reichstein Nielsen, 2014-03-06 09:26:11]

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
File sdk/lib/_internal/lib/isolate_helper.dart (right):

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
sdk/lib/_internal/lib/isolate_helper.dart:307: void addDoneListener(SendPort
response) {
On 2014/03/04 10:16:25, Søren Gjesse wrote:
> Use name responsePort as in isolate.dart.

Done.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/_internal/lib/is...
sdk/lib/_internal/lib/isolate_helper.dart:317: void removeDoneListener(SendPort
response) {
On 2014/03/04 10:16:25, Søren Gjesse wrote:
> Ditto.

Done.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:90: /**
On 2014/03/04 10:16:25, Søren Gjesse wrote:
> I think we should move the WARNING up as the first line in the comment, and
> extend it to also say that this is still experimental.

Done.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:155: * send a reply if the receiving isolate is
dead?
On 2014/03/04 10:16:25, Søren Gjesse wrote:
> Add the warning as for pause/resume.

Done.

https://codereview.chromium.org/179823002/diff/60001/sdk/lib/isolate/isolate....
sdk/lib/isolate/isolate.dart:157: void addOnExitListener(SendPort responsePort)
{
No. It's considered harmless. You don't leak any information from the isolate. 
If you have the isolate object, you can send control messages to it, including
this and the "ping" operation. To avoid that, don't give access to the isolate
(in this case, you can treat the control port itself as the capability needed
for these operations).