[vm] Simplify IsolateSpawnState
Move SpawnCount logic into SpawnIsolateTask, where it logically makes
more sense. This eliminates some code and also prevents
IsolateSpawnState from having lingering pointers to the parent
Isolate's fields after it might have been reclaimed.
Change-Id: If8d827f299ee90dd54faf896609a7acdfbb87608
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101628
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Matthew Dempsky <mdempsky@google.com>
diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc
index aed9937..c6280f1 100644
--- a/runtime/lib/isolate.cc
+++ b/runtime/lib/isolate.cc
@@ -113,14 +113,22 @@
class SpawnIsolateTask : public ThreadPool::Task {
public:
- explicit SpawnIsolateTask(IsolateSpawnState* state) : state_(state) {}
+ SpawnIsolateTask(Isolate* parent_isolate, IsolateSpawnState* state)
+ : parent_isolate_(parent_isolate), state_(state) {
+ parent_isolate->IncrementSpawnCount();
+ }
- virtual void Run() {
+ ~SpawnIsolateTask() override {
+ if (parent_isolate_) {
+ parent_isolate_->DecrementSpawnCount();
+ }
+ }
+
+ void Run() override {
// Create a new isolate.
char* error = NULL;
Dart_IsolateCreateCallback callback = Isolate::CreateCallback();
if (callback == NULL) {
- state_->DecrementSpawnCount();
ReportError(
"Isolate spawn is not supported by this Dart implementation\n");
delete state_;
@@ -136,8 +144,9 @@
Isolate* isolate = reinterpret_cast<Isolate*>((callback)(
state_->script_url(), name, nullptr, state_->package_config(),
- &api_flags, state_->init_data(), &error));
- state_->DecrementSpawnCount();
+ &api_flags, parent_isolate_->init_callback_data(), &error));
+ parent_isolate_->DecrementSpawnCount();
+ parent_isolate_ = nullptr;
if (isolate == NULL) {
ReportError(error);
delete state_;
@@ -152,7 +161,7 @@
isolate->set_origin_id(state_->origin_id());
}
MutexLocker ml(isolate->mutex());
- state_->set_isolate(reinterpret_cast<Isolate*>(isolate));
+ state_->set_isolate(isolate);
isolate->set_spawn_state(state_);
state_ = NULL;
if (isolate->is_runnable()) {
@@ -171,6 +180,7 @@
}
}
+ Isolate* parent_isolate_;
IsolateSpawnState* state_;
DISALLOW_COPY_AND_ASSIGN(SpawnIsolateTask);
@@ -229,21 +239,17 @@
debugName.IsNull() ? NULL : String2UTF8(debugName);
IsolateSpawnState* state = new IsolateSpawnState(
- port.Id(), isolate->origin_id(), isolate->init_callback_data(),
- String2UTF8(script_uri), func, &message_buffer,
- isolate->spawn_count_monitor(), isolate->spawn_count(),
- utf8_package_config, paused.value(), fatal_errors, on_exit_port,
- on_error_port, utf8_debug_name);
+ port.Id(), isolate->origin_id(), String2UTF8(script_uri), func,
+ &message_buffer, utf8_package_config, paused.value(), fatal_errors,
+ on_exit_port, on_error_port, utf8_debug_name);
// Since this is a call to Isolate.spawn, copy the parent isolate's code.
state->isolate_flags()->copy_parent_code = true;
- ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
+ ThreadPool::Task* spawn_task = new SpawnIsolateTask(isolate, state);
- isolate->IncrementSpawnCount();
if (!Dart::thread_pool()->Run(spawn_task)) {
// Running on the thread pool failed. Clean up everything.
- state->DecrementSpawnCount();
delete state;
state = NULL;
delete spawn_task;
@@ -355,10 +361,9 @@
debugName.IsNull() ? NULL : String2UTF8(debugName);
IsolateSpawnState* state = new IsolateSpawnState(
- port.Id(), isolate->init_callback_data(), canonical_uri,
- utf8_package_config, &arguments_buffer, &message_buffer,
- isolate->spawn_count_monitor(), isolate->spawn_count(), paused.value(),
- fatal_errors, on_exit_port, on_error_port, utf8_debug_name);
+ port.Id(), canonical_uri, utf8_package_config, &arguments_buffer,
+ &message_buffer, paused.value(), fatal_errors, on_exit_port,
+ on_error_port, utf8_debug_name);
// If we were passed a value then override the default flags state for
// checked mode.
@@ -370,12 +375,10 @@
// Since this is a call to Isolate.spawnUri, don't copy the parent's code.
state->isolate_flags()->copy_parent_code = false;
- ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
+ ThreadPool::Task* spawn_task = new SpawnIsolateTask(isolate, state);
- isolate->IncrementSpawnCount();
if (!Dart::thread_pool()->Run(spawn_task)) {
// Running on the thread pool failed. Clean up everything.
- state->DecrementSpawnCount();
delete state;
state = NULL;
delete spawn_task;
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 5aa112d..e80616a 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -2796,6 +2796,13 @@
spawn_count_++;
}
+void Isolate::DecrementSpawnCount() {
+ MonitorLocker ml(spawn_count_monitor_);
+ ASSERT(spawn_count_ > 0);
+ spawn_count_--;
+ ml.Notify();
+}
+
void Isolate::WaitForOutstandingSpawns() {
MonitorLocker ml(spawn_count_monitor_);
while (spawn_count_ > 0) {
@@ -2924,12 +2931,9 @@
IsolateSpawnState::IsolateSpawnState(Dart_Port parent_port,
Dart_Port origin_id,
- void* init_data,
const char* script_url,
const Function& func,
SerializedObjectBuffer* message_buffer,
- Monitor* spawn_count_monitor,
- intptr_t* spawn_count,
const char* package_config,
bool paused,
bool errors_are_fatal,
@@ -2939,7 +2943,6 @@
: isolate_(nullptr),
parent_port_(parent_port),
origin_id_(origin_id),
- init_data_(init_data),
on_exit_port_(on_exit_port),
on_error_port_(on_error_port),
script_url_(script_url),
@@ -2950,8 +2953,6 @@
debug_name_(debug_name),
serialized_args_(nullptr),
serialized_message_(message_buffer->StealMessage()),
- spawn_count_monitor_(spawn_count_monitor),
- spawn_count_(spawn_count),
paused_(paused),
errors_are_fatal_(errors_are_fatal) {
const Class& cls = Class::Handle(func.Owner());
@@ -2973,13 +2974,10 @@
}
IsolateSpawnState::IsolateSpawnState(Dart_Port parent_port,
- void* init_data,
const char* script_url,
const char* package_config,
SerializedObjectBuffer* args_buffer,
SerializedObjectBuffer* message_buffer,
- Monitor* spawn_count_monitor,
- intptr_t* spawn_count,
bool paused,
bool errors_are_fatal,
Dart_Port on_exit_port,
@@ -2988,7 +2986,6 @@
: isolate_(nullptr),
parent_port_(parent_port),
origin_id_(ILLEGAL_PORT),
- init_data_(init_data),
on_exit_port_(on_exit_port),
on_error_port_(on_error_port),
script_url_(script_url),
@@ -2999,8 +2996,6 @@
debug_name_(debug_name),
serialized_args_(args_buffer->StealMessage()),
serialized_message_(message_buffer->StealMessage()),
- spawn_count_monitor_(spawn_count_monitor),
- spawn_count_(spawn_count),
isolate_flags_(),
paused_(paused),
errors_are_fatal_(errors_are_fatal) {
@@ -3105,13 +3100,4 @@
return DeserializeMessage(thread, serialized_message_.get());
}
-void IsolateSpawnState::DecrementSpawnCount() {
- ASSERT(spawn_count_monitor_ != nullptr);
- ASSERT(spawn_count_ != nullptr);
- MonitorLocker ml(spawn_count_monitor_);
- ASSERT(*spawn_count_ > 0);
- *spawn_count_ = *spawn_count_ - 1;
- ml.Notify();
-}
-
} // namespace dart
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index 4535961..dbfe0ab 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -438,6 +438,7 @@
intptr_t* spawn_count() { return &spawn_count_; }
void IncrementSpawnCount();
+ void DecrementSpawnCount();
void WaitForOutstandingSpawns();
static void SetCreateCallback(Dart_IsolateCreateCallback cb) {
@@ -1154,12 +1155,9 @@
public:
IsolateSpawnState(Dart_Port parent_port,
Dart_Port origin_id,
- void* init_data,
const char* script_url,
const Function& func,
SerializedObjectBuffer* message_buffer,
- Monitor* spawn_count_monitor,
- intptr_t* spawn_count,
const char* package_config,
bool paused,
bool errorsAreFatal,
@@ -1167,13 +1165,10 @@
Dart_Port onError,
const char* debug_name);
IsolateSpawnState(Dart_Port parent_port,
- void* init_data,
const char* script_url,
const char* package_config,
SerializedObjectBuffer* args_buffer,
SerializedObjectBuffer* message_buffer,
- Monitor* spawn_count_monitor,
- intptr_t* spawn_count,
bool paused,
bool errorsAreFatal,
Dart_Port onExit,
@@ -1186,7 +1181,6 @@
Dart_Port parent_port() const { return parent_port_; }
Dart_Port origin_id() const { return origin_id_; }
- void* init_data() const { return init_data_; }
Dart_Port on_exit_port() const { return on_exit_port_; }
Dart_Port on_error_port() const { return on_error_port_; }
const char* script_url() const { return script_url_; }
@@ -1204,13 +1198,10 @@
RawInstance* BuildArgs(Thread* thread);
RawInstance* BuildMessage(Thread* thread);
- void DecrementSpawnCount();
-
private:
Isolate* isolate_;
Dart_Port parent_port_;
Dart_Port origin_id_;
- void* init_data_;
Dart_Port on_exit_port_;
Dart_Port on_error_port_;
const char* script_url_;
@@ -1222,11 +1213,6 @@
std::unique_ptr<Message> serialized_args_;
std::unique_ptr<Message> serialized_message_;
- // This counter tracks the number of outstanding calls to spawn by the parent
- // isolate.
- Monitor* spawn_count_monitor_;
- intptr_t* spawn_count_;
-
Dart_IsolateFlags isolate_flags_;
bool paused_;
bool errors_are_fatal_;
