Skip to content

Commit

Permalink
diagnostics_channel: fix diagnostics channel memory leak
Browse files Browse the repository at this point in the history
PR-URL: #45633
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
theanarkh authored and danielleadams committed Jan 3, 2023
1 parent 3fb5c6d commit 1a47a7b
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/diagnostics_channel.js
Expand Up @@ -122,6 +122,9 @@ function unsubscribe(name, subscription) {
}

channels[name].decRef();
if (channels[name].getRef() === 0) {
delete channels[name];
}
return true;
}

Expand Down
9 changes: 9 additions & 0 deletions src/node_util.cc
Expand Up @@ -294,6 +294,13 @@ void WeakReference::Get(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(weak_ref->target_.Get(isolate));
}

void WeakReference::GetRef(const FunctionCallbackInfo<Value>& args) {
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
Isolate* isolate = args.GetIsolate();
args.GetReturnValue().Set(
v8::Number::New(isolate, weak_ref->reference_count_));
}

void WeakReference::IncRef(const FunctionCallbackInfo<Value>& args) {
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
weak_ref->reference_count_++;
Expand Down Expand Up @@ -391,6 +398,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(ArrayBufferViewHasBuffer);
registry->Register(WeakReference::New);
registry->Register(WeakReference::Get);
registry->Register(WeakReference::GetRef);
registry->Register(WeakReference::IncRef);
registry->Register(WeakReference::DecRef);
registry->Register(GuessHandleType);
Expand Down Expand Up @@ -464,6 +472,7 @@ void Initialize(Local<Object> target,
WeakReference::kInternalFieldCount);
weak_ref->Inherit(BaseObject::GetConstructorTemplate(env));
SetProtoMethod(isolate, weak_ref, "get", WeakReference::Get);
SetProtoMethod(isolate, weak_ref, "getRef", WeakReference::GetRef);
SetProtoMethod(isolate, weak_ref, "incRef", WeakReference::IncRef);
SetProtoMethod(isolate, weak_ref, "decRef", WeakReference::DecRef);
SetConstructorFunction(context, target, "WeakReference", weak_ref);
Expand Down
1 change: 1 addition & 0 deletions src/node_util.h
Expand Up @@ -23,6 +23,7 @@ class WeakReference : public SnapshotableObject {
v8::Local<v8::Object> target);
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Get(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetRef(const v8::FunctionCallbackInfo<v8::Value>& args);
static void IncRef(const v8::FunctionCallbackInfo<v8::Value>& args);
static void DecRef(const v8::FunctionCallbackInfo<v8::Value>& args);

Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-diagnostics-channel-memory-leak.js
@@ -0,0 +1,24 @@
// Flags: --expose-gc
'use strict';

// This test ensures that diagnostic channel references aren't leaked.

require('../common');
const { ok } = require('assert');

const { subscribe, unsubscribe } = require('diagnostics_channel');

function noop() {}

const heapUsedBefore = process.memoryUsage().heapUsed;

for (let i = 0; i < 1000; i++) {
subscribe(String(i), noop);
unsubscribe(String(i), noop);
}

global.gc();

const heapUsedAfter = process.memoryUsage().heapUsed;

ok(heapUsedBefore >= heapUsedAfter);

0 comments on commit 1a47a7b

Please sign in to comment.