Memory leak in v8 CpuProfiler

Version: v8.4.0
Platform: Windows 10 / Linux x86 32/64bit
Subsystem: v8

Dear Node community,
I am filing this ticket to make you aware of a memory leak in v8::CpuProfiler introduced in v8 versions used by Node 7.x / 8.x. My company uses v8::CpuProfiler APIs to render periodic samples of the running application and I noticed a continuous RSS size increase with Node.js version 7.x and 8.x. Earlier versions are not affected.
I reported the issue a few weeks back in v8 monorail (https://bugs.chromium.org/p/v8/issues/detail?id=6623) but it did not receive too much attention so far.

/cc @nodejs/v8

/cc @nodejs/diagnostics

I can confirm the issue and came to the same conclusion, that it's the ProfilerListener that is leaking the memory. A straightforward patch (see below) breaks a number of V8 tests however so this needs some more digging.

diff --git a/deps/v8/src/log.cc b/deps/v8/src/log.cc
index 7f029d8..57cd522 100644
--- a/deps/v8/src/log.cc
+++ b/deps/v8/src/log.cc
@@ -1854,6 +1854,7 @@ void Logger::SetUpProfilerListener() {
 void Logger::TearDownProfilerListener() {
   if (profiler_listener_->HasObservers()) return;
   removeCodeEventListener(profiler_listener_.get());
+  profiler_listener_.reset();
 }
 
 sampler::Sampler* Logger::sampler() {

Turns out the ProfilerListener is still used after TearDownProfilerListener()...

I have a new patch that passes V8's and node's test suites but it's a little gross. I'll see if I can spruce it up before I send it upstream.

diff --git a/deps/v8/src/profiler/cpu-profiler.cc b/deps/v8/src/profiler/cpu-profiler.cc
index 85f9d5e..3e7c474 100644
--- a/deps/v8/src/profiler/cpu-profiler.cc
+++ b/deps/v8/src/profiler/cpu-profiler.cc
@@ -275,6 +275,8 @@ void CpuProfiler::set_sampling_interval(base::TimeDelta value) {
 void CpuProfiler::ResetProfiles() {
   profiles_.reset(new CpuProfilesCollection(isolate_));
   profiles_->set_cpu_profiler(this);
+  Logger* logger = isolate_->logger();
+  logger->profiler_listener_.reset();
 }

 void CpuProfiler::CreateEntriesForRuntimeCallStats() {

@bnoordhuis

@bnoordhuis thanks a lot for your effort! is there a chance that this patch will be included into one of the next Node.js releases?

I am seeing this issue too with node 7 and 8. I see that the bug is closed. Which node release is the fix present in ?

@interviewQ

@interviewQ It was just fixed in Node.js yesterday (#17512) so it will come with the next Node.js v8.x and v9.x. We no longer maintain v7.x.

Closing, since #17512 fixes this.

Actually Node 9.3.0 released earlier this week should already contain the fix (please give it a shot and report back). For Node 8 the fix will be in the next release.

Thanks for the response. Would this get fixed in node 7 ? Amaranth

…

On Thu, Dec 14, 2017 at 3:58 PM, Ali Ijaz Sheikh ***@***.***> wrote: Actually Node 9.3.0 released earlier this week should already contain the fix (please give it a shot and report back). For Node 8 the fix will be in the next release. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14894 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ASSkiQg8Ok5-eaj6NZ3aUuHlrmDMaYgvks5tAbZAgaJpZM4O6Wql> .

Sorry, I missed your earlier message that this will not be fixed in node 7. On Thu, Dec 14, 2017 at 4:10 PM, Amarnath Shanbhag < amarnathshanbhag@gmail.com> wrote:

…

Thanks for the response. Would this get fixed in node 7 ? Amaranth On Thu, Dec 14, 2017 at 3:58 PM, Ali Ijaz Sheikh ***@***.*** > wrote: > Actually Node 9.3.0 released earlier this week should already contain the > fix (please give it a shot and report back). For Node 8 the fix will be in > the next release. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#14894 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ASSkiQg8Ok5-eaj6NZ3aUuHlrmDMaYgvks5tAbZAgaJpZM4O6Wql> > . >

Navigation Menu

Repository navigation

Memory leak in v8 CpuProfiler #14894

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Memory leak in v8 CpuProfiler #14894

Description

Activity

mscdex commented on Aug 17, 2017

danielkhan commented on Aug 18, 2017

bnoordhuis commented on Aug 18, 2017

bnoordhuis commented on Aug 18, 2017

Hollerberg commented on Aug 31, 2017

interviewQ commented on Dec 14, 2017

TimothyGu commented on Dec 14, 2017

ofrobots commented on Dec 14, 2017

interviewQ commented on Dec 15, 2017

interviewQ commented on Dec 15, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions