Defer |Dart_Cleanup| till no isolates are in the middle of initialization.

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.

https://dart.dev

BSD 3-Clause "New" or "Revised" License

10.09k stars 1.56k forks source link

Defer |Dart_Cleanup| till no isolates are in the middle of initialization. #36502

Open chinmaygarde opened 5 years ago

chinmaygarde commented 5 years ago

Currently, isolates could be in the middle of initialization (and making dart_api.h calls) when the call the Dart_Cleanup is made on another thread. Embedders not resilient to unwind errors will run into crashes.

chinmaygarde commented 5 years ago

cc @rmacnak-google @a-siva

chinmaygarde commented 5 years ago

@rmacnak-google IIUC, your patch will make it so that that shutdown of Dart managed isolate is safe, but I still have to shut down engine managed isolates correct?

rmacnak-google commented 5 years ago

Dart_Cleanup will send an unwind error to all isolates and wait for

spawning of the internal-ish service and kernel isolates to finish
any Isolate.spawn/spawnUri that is in progress
isolates running on the VM's thread pool to return to the top of the message loop
isolates managing their own scheduling to check in to Dart_ShutdownIsolate

Provided the engine managed isolates respond to an unwind error (Dart_IsFatalError) by eventually calling Dart_ShutdownIsolate, the engine shouldn't need to perform its own shutdown broadcast.

chinmaygarde commented 5 years ago

any Isolate.spawn/spawnUri that is in progress

Does this mean waiting for the isolate create callback to return?

About the rest, sounds good and thanks for clarifying.