Skip to content

Commit

Permalink
Fix crash in code coverage generation.
Browse files Browse the repository at this point in the history
Separate the higher- and lower-level cleanup steps during isolate shutdown. The former (currently only code coverage and timeline) may need to allocate, but the latter should not. Add assertions.

(cc:zra, who's working on clean shutdown)

BUG=23848
R=rmacnak@google.com

Review URL: https://codereview.chromium.org//1236913006 .
  • Loading branch information
kodandersson committed Jul 17, 2015
1 parent d37ac8d commit ee49289
Showing 1 changed file with 28 additions and 9 deletions.
37 changes: 28 additions & 9 deletions runtime/vm/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1433,24 +1433,42 @@ void Isolate::Shutdown() {
}
#endif // DEBUG

// First, perform higher-level cleanup that may need to allocate.
{
// Ensure we have a zone and handle scope so that we can call VM functions.
StackZone stack_zone(this);
HandleScope handle_scope(this);

// Write out the coverage data if collection has been enabled.
CodeCoverage::Write(this);

if ((timeline_event_recorder_ != NULL) &&
(FLAG_timeline_trace_dir != NULL)) {
timeline_event_recorder_->WriteTo(FLAG_timeline_trace_dir);
}
}

// Remove this isolate from the list *before* we start tearing it down, to
// avoid exposing it in a state of decay.
RemoveIsolateFromList(this);

if (heap_ != NULL) {
// Wait for any concurrent GC tasks to finish before shutting down.
// TODO(koda): Support faster sweeper shutdown (e.g., after current page).
PageSpace* old_space = heap_->old_space();
MonitorLocker ml(old_space->tasks_lock());
while (old_space->tasks() > 0) {
ml.Wait();
}
}

// Create an area where we do have a zone and a handle scope so that we can
// call VM functions while tearing this isolate down.
// Then, proceed with low-level teardown.
{
// Ensure we have a zone and handle scope so that we can call VM functions,
// but we no longer allocate new heap objects.
StackZone stack_zone(this);
HandleScope handle_scope(this);
NoSafepointScope no_safepoint_scope;

if (compiler_stats_ != NULL) {
compiler_stats()->Print();
Expand All @@ -1474,9 +1492,6 @@ void Isolate::Shutdown() {
// Dump all accumulated timer data for the isolate.
timer_list_.ReportTimers();

// Write out the coverage data if collection has been enabled.
CodeCoverage::Write(this);

// Finalize any weak persistent handles with a non-null referent.
FinalizeWeakPersistentHandlesVisitor visitor;
api_state()->weak_persistent_handles().VisitHandles(&visitor);
Expand All @@ -1489,12 +1504,16 @@ void Isolate::Shutdown() {
OS::Print("[-] Stopping isolate:\n"
"\tisolate: %s\n", name());
}
}

if ((timeline_event_recorder_ != NULL) &&
(FLAG_timeline_trace_dir != NULL)) {
timeline_event_recorder_->WriteTo(FLAG_timeline_trace_dir);
}
#if defined(DEBUG)
// No concurrent sweeper tasks should be running at this point.
if (heap_ != NULL) {
PageSpace* old_space = heap_->old_space();
MonitorLocker ml(old_space->tasks_lock());
ASSERT(old_space->tasks() == 0);
}
#endif

// TODO(5411455): For now just make sure there are no current isolates
// as we are shutting down the isolate.
Expand Down

0 comments on commit ee49289

Please sign in to comment.