Skip to content

Commit e143a52

Browse files
liamappelbecommit-bot@chromium.org
liamappelbe
authored and
[email protected]
committed
Load isolate from parent's kernel in Isolate.spawn calls.
Bug: #6610 Change-Id: Icd6e1c5d6d4b64b611fc58333c051a8a9a50679d Reviewed-on: https://dart-review.googlesource.com/c/85724 Commit-Queue: Liam Appelbe <[email protected]> Reviewed-by: Vyacheslav Egorov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 4bda0fe commit e143a52

File tree

10 files changed

+157
-32
lines changed

10 files changed

+157
-32
lines changed

‎runtime/bin/isolate_data.cc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ IsolateData::IsolateData(const char* url,
2121
dependencies_(NULL),
2222
resolved_packages_config_(NULL),
2323
kernel_buffer_(NULL),
24-
kernel_buffer_size_(0),
25-
owns_kernel_buffer_(false) {
24+
kernel_buffer_size_(0) {
2625
if (package_root != NULL) {
2726
ASSERT(packages_file == NULL);
2827
this->package_root = strdup(package_root);
@@ -43,10 +42,6 @@ IsolateData::~IsolateData() {
4342
packages_file = NULL;
4443
free(resolved_packages_config_);
4544
resolved_packages_config_ = NULL;
46-
if (owns_kernel_buffer_) {
47-
ASSERT(kernel_buffer_ != NULL);
48-
free(kernel_buffer_);
49-
}
5045
kernel_buffer_ = NULL;
5146
kernel_buffer_size_ = 0;
5247
delete app_snapshot_;

‎runtime/bin/isolate_data.h

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
#ifndef RUNTIME_BIN_ISOLATE_DATA_H_
66
#define RUNTIME_BIN_ISOLATE_DATA_H_
77

8+
#include <memory>
9+
#include <utility>
10+
811
#include "include/dart_api.h"
912
#include "platform/assert.h"
1013
#include "platform/globals.h"
@@ -40,13 +43,37 @@ class IsolateData {
4043
char* package_root;
4144
char* packages_file;
4245

43-
const uint8_t* kernel_buffer() const { return kernel_buffer_; }
46+
const std::shared_ptr<uint8_t>& kernel_buffer() const {
47+
return kernel_buffer_;
48+
}
49+
4450
intptr_t kernel_buffer_size() const { return kernel_buffer_size_; }
45-
void set_kernel_buffer(uint8_t* buffer, intptr_t size, bool take_ownership) {
46-
ASSERT(kernel_buffer_ == NULL);
47-
kernel_buffer_ = buffer;
51+
52+
// Associate the given kernel buffer with this IsolateData without giving it
53+
// ownership of the buffer.
54+
void SetKernelBufferUnowned(uint8_t* buffer, intptr_t size) {
55+
ASSERT(kernel_buffer_.get() == NULL);
56+
kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, FreeUnownedKernelBuffer);
57+
kernel_buffer_size_ = size;
58+
}
59+
60+
// Associate the given kernel buffer with this IsolateData and give it
61+
// ownership of the buffer. This IsolateData is the first one to own the
62+
// buffer.
63+
void SetKernelBufferNewlyOwned(uint8_t* buffer, intptr_t size) {
64+
ASSERT(kernel_buffer_.get() == NULL);
65+
kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, free);
66+
kernel_buffer_size_ = size;
67+
}
68+
69+
// Associate the given kernel buffer with this IsolateData and give it
70+
// ownership of the buffer. The buffer is already owned by another
71+
// IsolateData.
72+
void SetKernelBufferAlreadyOwned(std::shared_ptr<uint8_t> buffer,
73+
intptr_t size) {
74+
ASSERT(kernel_buffer_.get() == NULL);
75+
kernel_buffer_ = std::move(buffer);
4876
kernel_buffer_size_ = size;
49-
owns_kernel_buffer_ = take_ownership;
5077
}
5178

5279
void UpdatePackagesFile(const char* packages_file_) {
@@ -91,9 +118,10 @@ class IsolateData {
91118
AppSnapshot* app_snapshot_;
92119
MallocGrowableArray<char*>* dependencies_;
93120
char* resolved_packages_config_;
94-
uint8_t* kernel_buffer_;
121+
std::shared_ptr<uint8_t> kernel_buffer_;
95122
intptr_t kernel_buffer_size_;
96-
bool owns_kernel_buffer_;
123+
124+
static void FreeUnownedKernelBuffer(uint8_t*) {}
97125

98126
DISALLOW_COPY_AND_ASSIGN(IsolateData);
99127
};

‎runtime/bin/main.cc

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <stdio.h>
66
#include <stdlib.h>
77
#include <string.h>
8+
#include <memory>
89

910
#include "include/bin/dart_io_api.h"
1011
#include "include/dart_api.h"
@@ -213,7 +214,7 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
213214
#if !defined(DART_PRECOMPILED_RUNTIME)
214215
IsolateData* isolate_data =
215216
reinterpret_cast<IsolateData*>(Dart_IsolateData(isolate));
216-
const uint8_t* kernel_buffer = isolate_data->kernel_buffer();
217+
const uint8_t* kernel_buffer = isolate_data->kernel_buffer().get();
217218
intptr_t kernel_buffer_size = isolate_data->kernel_buffer_size();
218219
#endif
219220

@@ -274,9 +275,8 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
274275
Dart_ShutdownIsolate();
275276
return NULL;
276277
}
277-
isolate_data->set_kernel_buffer(application_kernel_buffer,
278-
application_kernel_buffer_size,
279-
true /*take ownership*/);
278+
isolate_data->SetKernelBufferNewlyOwned(application_kernel_buffer,
279+
application_kernel_buffer_size);
280280
kernel_buffer = application_kernel_buffer;
281281
kernel_buffer_size = application_kernel_buffer_size;
282282
}
@@ -433,9 +433,9 @@ static Dart_Isolate CreateAndSetupKernelIsolate(const char* script_uri,
433433
dfe.LoadKernelService(&kernel_service_buffer, &kernel_service_buffer_size);
434434
ASSERT(kernel_service_buffer != NULL);
435435
isolate_data = new IsolateData(uri, package_root, packages_config, NULL);
436-
isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
437-
kernel_service_buffer_size,
438-
false /* take_ownership */);
436+
isolate_data->SetKernelBufferUnowned(
437+
const_cast<uint8_t*>(kernel_service_buffer),
438+
kernel_service_buffer_size);
439439
isolate = Dart_CreateIsolateFromKernel(
440440
DART_KERNEL_ISOLATE_NAME, main, kernel_service_buffer,
441441
kernel_service_buffer_size, flags, isolate_data, error);
@@ -527,11 +527,13 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
527527
const char* package_root,
528528
const char* packages_config,
529529
Dart_IsolateFlags* flags,
530+
void* callback_data,
530531
char** error,
531532
int* exit_code) {
532533
int64_t start = Dart_TimelineGetMicros();
533534
ASSERT(script_uri != NULL);
534535
uint8_t* kernel_buffer = NULL;
536+
std::shared_ptr<uint8_t> parent_kernel_buffer;
535537
intptr_t kernel_buffer_size = 0;
536538
AppSnapshot* app_snapshot = NULL;
537539

@@ -565,16 +567,30 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
565567
&isolate_snapshot_data, &isolate_snapshot_instructions);
566568
}
567569
}
568-
if (!isolate_run_app_snapshot) {
570+
571+
if (flags->copy_parent_code && callback_data) {
572+
IsolateData* parent_isolate_data =
573+
reinterpret_cast<IsolateData*>(callback_data);
574+
parent_kernel_buffer = parent_isolate_data->kernel_buffer();
575+
kernel_buffer = parent_kernel_buffer.get();
576+
kernel_buffer_size = parent_isolate_data->kernel_buffer_size();
577+
}
578+
579+
if (kernel_buffer == NULL && !isolate_run_app_snapshot) {
569580
dfe.ReadScript(script_uri, &kernel_buffer, &kernel_buffer_size);
570581
}
571582
#endif // !defined(DART_PRECOMPILED_RUNTIME)
572583

573584
IsolateData* isolate_data =
574585
new IsolateData(script_uri, package_root, packages_config, app_snapshot);
575586
if (kernel_buffer != NULL) {
576-
isolate_data->set_kernel_buffer(kernel_buffer, kernel_buffer_size,
577-
true /*take ownership*/);
587+
if (parent_kernel_buffer) {
588+
isolate_data->SetKernelBufferAlreadyOwned(std::move(parent_kernel_buffer),
589+
kernel_buffer_size);
590+
} else {
591+
isolate_data->SetKernelBufferNewlyOwned(kernel_buffer,
592+
kernel_buffer_size);
593+
}
578594
}
579595
if (is_main_isolate && (Options::depfile() != NULL)) {
580596
isolate_data->set_dependencies(new MallocGrowableArray<char*>());
@@ -640,7 +656,7 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
640656
const char* package_root,
641657
const char* package_config,
642658
Dart_IsolateFlags* flags,
643-
void* data,
659+
void* callback_data,
644660
char** error) {
645661
// The VM should never call the isolate helper with a NULL flags.
646662
ASSERT(flags != NULL);
@@ -667,8 +683,8 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
667683
}
668684
bool is_main_isolate = false;
669685
return CreateIsolateAndSetupHelper(is_main_isolate, script_uri, main,
670-
package_root, package_config, flags, error,
671-
&exit_code);
686+
package_root, package_config, flags,
687+
callback_data, error, &exit_code);
672688
}
673689

674690
char* BuildIsolateName(const char* script_name, const char* func_name) {
@@ -802,7 +818,8 @@ bool RunMainIsolate(const char* script_name, CommandLineOptions* dart_options) {
802818

803819
Dart_Isolate isolate = CreateIsolateAndSetupHelper(
804820
is_main_isolate, script_name, "main", Options::package_root(),
805-
Options::packages_file(), &flags, &error, &exit_code);
821+
Options::packages_file(), &flags, NULL /* callback_data */, &error,
822+
&exit_code);
806823

807824
if (isolate == NULL) {
808825
delete[] isolate_name;

‎runtime/bin/run_vm_tests.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
166166
ASSERT(kernel_service_buffer != NULL);
167167
isolate_data =
168168
new bin::IsolateData(script_uri, package_root, packages_config, NULL);
169-
isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
170-
kernel_service_buffer_size,
171-
false /* take_ownership */);
169+
isolate_data->SetKernelBufferUnowned(
170+
const_cast<uint8_t*>(kernel_service_buffer),
171+
kernel_service_buffer_size);
172172
isolate = Dart_CreateIsolateFromKernel(
173173
script_uri, main, kernel_service_buffer, kernel_service_buffer_size,
174174
flags, isolate_data, error);

‎runtime/include/dart_api.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ typedef struct {
553553
* for each part.
554554
*/
555555

556-
#define DART_FLAGS_CURRENT_VERSION (0x0000000a)
556+
#define DART_FLAGS_CURRENT_VERSION (0x0000000b)
557557

558558
typedef struct {
559559
int32_t version;
@@ -564,6 +564,7 @@ typedef struct {
564564
Dart_QualifiedFunctionName* entry_points;
565565
bool load_vmservice_library;
566566
bool unsafe_trust_strong_mode_types;
567+
bool copy_parent_code;
567568
} Dart_IsolateFlags;
568569

569570
/**

‎runtime/lib/isolate.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,10 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 10) {
230230
isolate->spawn_count_monitor(), isolate->spawn_count(),
231231
utf8_package_root, utf8_package_config, paused.value(), fatal_errors,
232232
on_exit_port, on_error_port);
233+
234+
// Since this is a call to Isolate.spawn, copy the parent isolate's code.
235+
state->isolate_flags()->copy_parent_code = true;
236+
233237
ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
234238

235239
isolate->IncrementSpawnCount();
@@ -357,6 +361,9 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 12) {
357361
flags->enable_asserts = checked.value();
358362
}
359363

364+
// Since this is a call to Isolate.spawnUri, don't copy the parent's code.
365+
state->isolate_flags()->copy_parent_code = false;
366+
360367
ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
361368

362369
isolate->IncrementSpawnCount();

‎runtime/vm/isolate.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,7 @@ void Isolate::FlagsInitialize(Dart_IsolateFlags* api_flags) {
779779
#undef INIT_FROM_FLAG
780780
api_flags->entry_points = NULL;
781781
api_flags->load_vmservice_library = false;
782+
api_flags->copy_parent_code = false;
782783
}
783784

784785
void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
@@ -789,6 +790,7 @@ void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
789790
#undef INIT_FROM_FIELD
790791
api_flags->entry_points = NULL;
791792
api_flags->load_vmservice_library = should_load_vmservice();
793+
api_flags->copy_parent_code = false;
792794
}
793795

794796
void Isolate::FlagsCopyFrom(const Dart_IsolateFlags& api_flags) {

‎runtime/vm/isolate.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ typedef FixedCache<intptr_t, CatchEntryMovesRefPtr, 16> CatchEntryMovesCache;
133133
// List of Isolate flags with corresponding members of Dart_IsolateFlags and
134134
// corresponding global command line flags.
135135
//
136-
// V(when, name, Dart_IsolateFlags-member-name, command-line-flag-name)
136+
// V(when, name, bit-name, Dart_IsolateFlags-name, command-line-flag-name)
137137
//
138138
#define ISOLATE_FLAG_LIST(V) \
139139
V(NONPRODUCT, asserts, EnableAsserts, enable_asserts, FLAG_enable_asserts) \

‎tests/lib_2/isolate/issue_6610_test.dart

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Testing that Isolate.spawn copies the source code of the parent isolate,
6+
// rather than rereading the parent's source URI.
7+
// https://github.com/dart-lang/sdk/issues/6610
8+
9+
// Isolate structure:
10+
// Root 1 -> Branch 1 -> Leaf 1
11+
// /
12+
// main
13+
// \
14+
// Root 2 -> Branch 2 -> Leaf 2
15+
16+
library spawn_tests;
17+
18+
import "dart:io";
19+
import 'dart:isolate';
20+
import 'package:expect/expect.dart';
21+
22+
void main() {
23+
HttpServer.bind("127.0.0.1", 0).then((server) {
24+
var count = 0;
25+
server.listen((HttpRequest request) {
26+
++count;
27+
request.response.write("""
28+
import 'dart:isolate';
29+
30+
void main(_, SendPort port) {
31+
root(port);
32+
}
33+
34+
void root(SendPort port) {
35+
port.send("Root ${count}");
36+
Isolate.spawn(branch, port);
37+
}
38+
39+
void branch(SendPort port) {
40+
port.send("Branch ${count}");
41+
Isolate.spawn(leaf, port);
42+
}
43+
44+
void leaf(SendPort port) {
45+
port.send("Leaf ${count}");
46+
}
47+
""");
48+
request.response.close();
49+
});
50+
51+
ReceivePort port = new ReceivePort();
52+
var messageSet = Set();
53+
port.listen((message) {
54+
messageSet.add(message);
55+
if (messageSet.length >= 6) {
56+
server.close();
57+
port.close();
58+
Expect.setEquals([
59+
"Root 1",
60+
"Root 2",
61+
"Branch 1",
62+
"Branch 2",
63+
"Leaf 1",
64+
"Leaf 2",
65+
], messageSet);
66+
}
67+
});
68+
69+
Isolate.spawnUri(
70+
Uri.parse("http://127.0.0.1:${server.port}"), [], port.sendPort);
71+
Isolate.spawnUri(
72+
Uri.parse("http://127.0.0.1:${server.port}"), [], port.sendPort);
73+
});
74+
}

‎tests/lib_2/lib_2.status

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ isolate/isolate_complex_messages_test: Skip # Isolate.spawnUri
212212
isolate/issue_21398_parent_isolate1_test: Skip # Isolate.spawnUri
213213
isolate/issue_21398_parent_isolate_test: Skip # Isolate.spawnUri
214214
isolate/issue_24243_parent_isolate_test: Skip # Isolate.spawnUri
215+
isolate/issue_6610_test: Skip # Isolate.spawnUri
215216
isolate/mandel_isolate_test: Skip # Isolate.spawnUri
216217
isolate/message2_test: Skip # Isolate.spawnUri
217218
isolate/message_test: Skip # Isolate.spawnUri

0 commit comments

Comments
 (0)