From 0ac0edb70760f58315cdf38c6140bc067ef2d4e5 Mon Sep 17 00:00:00 2001 From: "yurys@chromium.org" Date: Mon, 1 Jul 2013 12:32:52 +0000 Subject: [PATCH] Test that profiler is stopped when isolate is being disposed The only way to get v8::CpuProfiler instance in the V8 public API is to call v8::Iolate::GetCpuProfiler(). The method will return NULL if the isolate has not been initialized yet or has been torn down already. It is the client's reponsibility to make sure that CPU profiling has been stopped before disposing of the isolate. This CL adds a test for this and several ASSRTS enforcing that assumptions. This allowed to be sure that heap is always setup when CPU profiling is being started. Based on that the number of places where already compiled functions are reported to the profiler event processor boils down to the single place (CpuProfiler::StartProcessorIfNotStarted). I'm going to rely on this assumption in further changes. BUG=None R=loislo@chromium.org, yangguo@chromium.org Review URL: https://codereview.chromium.org/18336002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15415 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 5 +++-- src/cpu-profiler.cc | 19 ++++++++++--------- test/cctest/test-cpu-profiler.cc | 20 ++++++++++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/include/v8.h b/include/v8.h index c4b22e3..cefc87a 100644 --- a/include/v8.h +++ b/include/v8.h @@ -3993,8 +3993,9 @@ class V8EXPORT Isolate { HeapProfiler* GetHeapProfiler(); /** - * Returns CPU profiler for this isolate. Will return NULL until the isolate - * is initialized. + * Returns CPU profiler for this isolate. Will return NULL unless the isolate + * is initialized. It is the embedder's responsibility to stop all CPU + * profiling activities if it has started any. */ CpuProfiler* GetCpuProfiler(); diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.cc index 512f7d5..b6cfbc5 100644 --- a/src/cpu-profiler.cc +++ b/src/cpu-profiler.cc @@ -405,6 +405,7 @@ CpuProfiler::CpuProfiler(Isolate* isolate, CpuProfiler::~CpuProfiler() { + ASSERT(!is_profiling_); delete token_enumerator_; delete profiles_; } @@ -430,23 +431,23 @@ void CpuProfiler::StartProfiling(String* title, bool record_samples) { void CpuProfiler::StartProcessorIfNotStarted() { if (processor_ == NULL) { + Logger* logger = isolate_->logger(); // Disable logging when using the new implementation. - saved_logging_nesting_ = isolate_->logger()->logging_nesting_; - isolate_->logger()->logging_nesting_ = 0; + saved_logging_nesting_ = logger->logging_nesting_; + logger->logging_nesting_ = 0; generator_ = new ProfileGenerator(profiles_); processor_ = new ProfilerEventsProcessor(generator_); is_profiling_ = true; processor_->StartSynchronously(); // Enumerate stuff we already have in the heap. - if (isolate_->heap()->HasBeenSetUp()) { - if (!FLAG_prof_browser_mode) { - isolate_->logger()->LogCodeObjects(); - } - isolate_->logger()->LogCompiledFunctions(); - isolate_->logger()->LogAccessorCallbacks(); + ASSERT(isolate_->heap()->HasBeenSetUp()); + if (!FLAG_prof_browser_mode) { + logger->LogCodeObjects(); } + logger->LogCompiledFunctions(); + logger->LogAccessorCallbacks(); // Enable stack sampling. - Sampler* sampler = isolate_->logger()->sampler(); + Sampler* sampler = logger->sampler(); sampler->IncreaseProfilingDepth(); if (!sampler->IsActive()) { sampler->Start(); diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index fb0566d..251a4a5 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -450,6 +450,26 @@ TEST(DeleteCpuProfileDifferentTokens) { } +TEST(GetProfilerWhenIsolateIsNotInitialized) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + CHECK(i::Isolate::Current()->IsDefaultIsolate()); + CHECK(!i::Isolate::Current()->IsInitialized()); + CHECK_EQ(NULL, isolate->GetCpuProfiler()); + { + v8::Isolate::Scope isolateScope(isolate); + LocalContext env; + v8::HandleScope scope(isolate); + CHECK_NE(NULL, isolate->GetCpuProfiler()); + isolate->GetCpuProfiler()->StartCpuProfiling(v8::String::New("Test")); + isolate->GetCpuProfiler()->StopCpuProfiling(v8::String::New("Test")); + } + CHECK(i::Isolate::Current()->IsInitialized()); + CHECK_NE(NULL, isolate->GetCpuProfiler()); + isolate->Dispose(); + CHECK_EQ(NULL, isolate->GetCpuProfiler()); +} + + static bool ContainsString(v8::Handle string, const Vector >& vector) { for (int i = 0; i < vector.length(); i++) { -- 2.7.4