Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 3c4c62d

Browse files
bkonyicommit-bot@chromium.org
bkonyi
authored and
[email protected]
committed
Reland "[VM] Dart_Initialize no longer crashes after Dart_Cleanup"
This is a reland of 519ee90 Original change's description: > [VM] Dart_Initialize no longer crashes after Dart_Cleanup > > Change-Id: I3cfdab9553aad045f024b6f9aec0b40b08234007 > Reviewed-on: https://dart-review.googlesource.com/75786 > Reviewed-by: Ryan Macnak <[email protected]> > Commit-Queue: Ben Konyi <[email protected]> Change-Id: Icdd901bf1ab0b675cc5f1497061a2b7b8a05d956 Reviewed-on: https://dart-review.googlesource.com/76561 Commit-Queue: Ben Konyi <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent e31f9fc commit 3c4c62d

35 files changed

+315
-72
lines changed

‎runtime/bin/run_vm_tests.cc‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,15 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
150150
isolate = Dart_CreateIsolate(
151151
DART_KERNEL_ISOLATE_NAME, main, isolate_snapshot_data,
152152
isolate_snapshot_instructions, NULL, NULL, flags, isolate_data, error);
153+
if (*error != NULL) {
154+
free(*error);
155+
*error = NULL;
156+
}
153157
}
154158
if (isolate == NULL) {
159+
delete isolate_data;
160+
isolate_data = NULL;
161+
155162
bin::dfe.Init();
156163
bin::dfe.LoadKernelService(&kernel_service_buffer,
157164
&kernel_service_buffer_size);
@@ -286,6 +293,10 @@ static int Main(int argc, const char** argv) {
286293
ASSERT(error == NULL);
287294
}
288295

296+
TesterState::vm_snapshot_data = dart::bin::vm_snapshot_data;
297+
TesterState::create_callback = CreateIsolateAndSetup;
298+
TesterState::cleanup_callback = CleanupIsolate;
299+
289300
error = Dart::InitOnce(
290301
dart::bin::vm_snapshot_data, dart::bin::vm_snapshot_instructions,
291302
CreateIsolateAndSetup /* create */, NULL /* shutdown */,
@@ -304,9 +315,10 @@ static int Main(int argc, const char** argv) {
304315
error = Dart::Cleanup();
305316
ASSERT(error == NULL);
306317

318+
TestCaseBase::RunAllRaw();
319+
307320
bin::EventHandler::Stop();
308321

309-
TestCaseBase::RunAllRaw();
310322
// Print a warning message if no tests or benchmarks were matched.
311323
if (run_matches == 0) {
312324
bin::Log::PrintErr("No tests matched: %s\n", run_filter);

‎runtime/tests/vm/vm.status‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ dart/data_uri_import_test/wrongmime: Crash
146146
[ $arch == x64 && ($compiler == dartk || $compiler == dartkb) && $system == windows && $strong ]
147147
cc/Profiler_BasicSourcePosition: Fail # http://dartbug.com/33224
148148
cc/Profiler_CodeTicks: Fail # dartbug.com/33337
149-
150149
[ $arch == ia32 && $compiler != dartk && $compiler != dartkp && $compiler != dartkb && $system == windows && $runtime == vm && $mode == debug ]
151150
cc/BitTestImmediate: Crash # dartbug.com/34252
152151

‎runtime/vm/code_observers.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ bool CodeObservers::AreActive() {
4747
return false;
4848
}
4949

50-
void CodeObservers::DeleteAll() {
50+
void CodeObservers::Cleanup() {
5151
for (intptr_t i = 0; i < observers_length_; i++) {
5252
delete observers_[i];
5353
}
@@ -57,8 +57,9 @@ void CodeObservers::DeleteAll() {
5757
}
5858

5959
void CodeObservers::InitOnce() {
60-
ASSERT(mutex_ == NULL);
61-
mutex_ = new Mutex();
60+
if (mutex_ == NULL) {
61+
mutex_ = new Mutex();
62+
}
6263
ASSERT(mutex_ != NULL);
6364
OS::RegisterCodeObservers();
6465
}

‎runtime/vm/code_observers.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class CodeObservers : public AllStatic {
6868
// Returns true if there is at least one active code observer.
6969
static bool AreActive();
7070

71-
static void DeleteAll();
71+
static void Cleanup();
7272

7373
static Mutex* mutex() { return mutex_; }
7474

‎runtime/vm/compiler_test.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ TEST_CASE(EvalExpression) {
195195
val = Instance::Cast(obj).EvaluateCompiledExpression(
196196
receiver_cls, kernel_bytes, kernel_length, Array::empty_array(),
197197
Array::empty_array(), TypeArguments::null_type_arguments());
198+
free(const_cast<uint8_t*>(kernel_bytes));
198199
}
199200
EXPECT(!val.IsNull());
200201
EXPECT(!val.IsError());

‎runtime/vm/dart.cc‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ char* Dart::InitOnce(const uint8_t* vm_isolate_snapshot,
176176
Api::InitOnce();
177177
NativeSymbolResolver::InitOnce();
178178
NOT_IN_PRODUCT(Profiler::InitOnce());
179-
SemiSpace::InitOnce();
179+
SemiSpace::Init();
180180
NOT_IN_PRODUCT(Metric::InitOnce());
181181
StoreBuffer::InitOnce();
182182
MarkingStack::InitOnce();
@@ -454,6 +454,7 @@ const char* Dart::Cleanup() {
454454
WaitForIsolateShutdown();
455455

456456
IdleNotifier::Stop();
457+
457458
// Shutdown the thread pool. On return, all thread pool threads have exited.
458459
if (FLAG_trace_shutdown) {
459460
OS::PrintErr("[+%" Pd64 "ms] SHUTDOWN: Deleting thread pool\n",
@@ -462,6 +463,10 @@ const char* Dart::Cleanup() {
462463
delete thread_pool_;
463464
thread_pool_ = NULL;
464465

466+
Api::Cleanup();
467+
delete predefined_handles_;
468+
predefined_handles_ = NULL;
469+
465470
// Disable creation of any new OSThread structures which means no more new
466471
// threads can do an EnterIsolate. This must come after isolate shutdown
467472
// because new threads may need to be spawned to shutdown the isolates.
@@ -485,12 +490,18 @@ const char* Dart::Cleanup() {
485490
ShutdownIsolate();
486491
vm_isolate_ = NULL;
487492
ASSERT(Isolate::IsolateListLength() == 0);
493+
PortMap::Cleanup();
488494
IdleNotifier::Cleanup();
489-
490495
TargetCPUFeatures::Cleanup();
491496
MarkingStack::ShutDown();
492497
StoreBuffer::ShutDown();
493-
498+
MarkingStack::ShutDown();
499+
Object::Cleanup();
500+
SemiSpace::Cleanup();
501+
#if !defined(DART_PRECOMPILED_RUNTIME)
502+
// Stubs are generated when not precompiled, clean them up.
503+
StubCode::Cleanup();
504+
#endif
494505
// Delete the current thread's TLS and set it's TLS to null.
495506
// If it is the last thread then the destructor would call
496507
// OSThread::Cleanup.
@@ -506,7 +517,7 @@ const char* Dart::Cleanup() {
506517
OS::PrintErr("[+%" Pd64 "ms] SHUTDOWN: Deleting code observers\n",
507518
UptimeMillis());
508519
}
509-
NOT_IN_PRODUCT(CodeObservers::DeleteAll());
520+
NOT_IN_PRODUCT(CodeObservers::Cleanup());
510521
if (FLAG_support_timeline) {
511522
if (FLAG_trace_shutdown) {
512523
OS::PrintErr("[+%" Pd64 "ms] SHUTDOWN: Shutting down timeline\n",

‎runtime/vm/dart_api_impl.cc‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,8 +479,9 @@ ApiLocalScope* Api::TopScope(Thread* thread) {
479479
}
480480

481481
void Api::InitOnce() {
482-
ASSERT(api_native_key_ == kUnsetThreadLocalKey);
483-
api_native_key_ = OSThread::CreateThreadLocal();
482+
if (api_native_key_ == kUnsetThreadLocalKey) {
483+
api_native_key_ = OSThread::CreateThreadLocal();
484+
}
484485
ASSERT(api_native_key_ != kUnsetThreadLocalKey);
485486
}
486487

@@ -511,6 +512,13 @@ void Api::InitHandles() {
511512
empty_string_handle_ = InitNewReadOnlyApiHandle(Symbols::Empty().raw());
512513
}
513514

515+
void Api::Cleanup() {
516+
true_handle_ = NULL;
517+
false_handle_ = NULL;
518+
null_handle_ = NULL;
519+
empty_string_handle_ = NULL;
520+
}
521+
514522
bool Api::StringGetPeerHelper(NativeArguments* arguments,
515523
int arg_index,
516524
void** peer) {

‎runtime/vm/dart_api_impl.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,9 @@ class Api : AllStatic {
244244
// Allocates handles for objects in the VM isolate.
245245
static void InitHandles();
246246

247+
// Cleanup
248+
static void Cleanup();
249+
247250
// Helper function to get the peer value of an external string object.
248251
static bool StringGetPeerHelper(NativeArguments* args,
249252
int arg_index,

‎runtime/vm/dart_api_impl_test.cc‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "vm/dart_api_impl.h"
66
#include "bin/builtin.h"
7+
#include "bin/dartutils.h"
78
#include "include/dart_api.h"
89
#include "include/dart_native_api.h"
910
#include "include/dart_tools_api.h"
@@ -12,6 +13,7 @@
1213
#include "platform/utils.h"
1314
#include "vm/class_finalizer.h"
1415
#include "vm/compiler/jit/compiler.h"
16+
#include "vm/dart.h"
1517
#include "vm/dart_api_state.h"
1618
#include "vm/debugger_api_impl_test.h"
1719
#include "vm/heap/verifier.h"
@@ -26,6 +28,35 @@ DECLARE_FLAG(bool, use_dart_frontend);
2628

2729
#ifndef PRODUCT
2830

31+
UNIT_TEST_CASE(DartAPI_DartInitializeAfterCleanup) {
32+
Dart_InitializeParams params;
33+
memset(&params, 0, sizeof(Dart_InitializeParams));
34+
params.version = DART_INITIALIZE_PARAMS_CURRENT_VERSION;
35+
params.vm_snapshot_data = TesterState::vm_snapshot_data;
36+
params.create = TesterState::create_callback;
37+
params.shutdown = TesterState::shutdown_callback;
38+
params.cleanup = TesterState::cleanup_callback;
39+
params.start_kernel_isolate = true;
40+
41+
// Reinitialize and ensure we can execute Dart code.
42+
EXPECT(Dart_Initialize(&params) == NULL);
43+
{
44+
TestIsolateScope scope;
45+
const char* kScriptChars =
46+
"int testMain() {\n"
47+
" return 42;\n"
48+
"}\n";
49+
Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL);
50+
EXPECT_VALID(lib);
51+
Dart_Handle result = Dart_Invoke(lib, NewString("testMain"), 0, NULL);
52+
EXPECT_VALID(result);
53+
int64_t value = 0;
54+
EXPECT_VALID(Dart_IntegerToInt64(result, &value));
55+
EXPECT_EQ(42, value);
56+
}
57+
EXPECT(Dart_Cleanup() == NULL);
58+
}
59+
2960
TEST_CASE(DartAPI_ErrorHandleBasics) {
3061
const char* kScriptChars =
3162
"void testMain() {\n"
@@ -57,7 +88,6 @@ TEST_CASE(DartAPI_ErrorHandleBasics) {
5788
EXPECT(Dart_IsError(Dart_ErrorGetException(instance)));
5889
EXPECT(Dart_IsError(Dart_ErrorGetException(error)));
5990
EXPECT_VALID(Dart_ErrorGetException(exception));
60-
6191
EXPECT(Dart_IsError(Dart_ErrorGetStackTrace(instance)));
6292
EXPECT(Dart_IsError(Dart_ErrorGetStackTrace(error)));
6393
EXPECT_VALID(Dart_ErrorGetStackTrace(exception));
@@ -3539,6 +3569,7 @@ static Dart_Handle LoadScript(const char* url_str, const char* source) {
35393569
if (error != NULL) {
35403570
return Dart_NewApiError(error);
35413571
}
3572+
TestCaseBase::AddToKernelBuffers(kernel_buffer);
35423573
return Dart_LoadScriptFromKernel(kernel_buffer, kernel_buffer_size);
35433574
}
35443575
}

‎runtime/vm/debugger_api_impl_test.cc‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -268,14 +268,17 @@ DART_EXPORT Dart_Handle Dart_EvaluateStaticExpr(Dart_Handle lib_handle,
268268

269269
const uint8_t* kernel_bytes = compilation_result.kernel;
270270
intptr_t kernel_length = compilation_result.kernel_size;
271-
return Api::NewHandle(T, lib.EvaluateCompiledExpression(
272-
kernel_bytes, kernel_length,
273-
/* type_definitions= */
274-
Array::empty_array(),
275-
/* param_values= */
276-
Array::empty_array(),
277-
/* type_param_values= */
278-
TypeArguments::null_type_arguments()));
271+
Dart_Handle result = Api::NewHandle(
272+
T,
273+
lib.EvaluateCompiledExpression(kernel_bytes, kernel_length,
274+
/* type_definitions= */
275+
Array::empty_array(),
276+
/* param_values= */
277+
Array::empty_array(),
278+
/* type_param_values= */
279+
TypeArguments::null_type_arguments()));
280+
free(const_cast<uint8_t*>(kernel_bytes));
281+
return result;
279282
}
280283
}
281284

0 commit comments

Comments
 (0)