From 1999ee7cdc0ffec3cc15e308217a47b505583ef6 Mon Sep 17 00:00:00 2001 From: Roch Devost Date: Mon, 13 Aug 2018 16:43:17 -0400 Subject: [PATCH] fix memory leak when using scopes with recursive timers (#227) --- src/scope/context.js | 5 ++++- src/scope/context_execution.js | 14 ++++++++++---- test/leak/scope_manager.js | 16 +++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/scope/context.js b/src/scope/context.js index f2c04f723ba..c586b4e8cdd 100644 --- a/src/scope/context.js +++ b/src/scope/context.js @@ -23,9 +23,12 @@ class Context { } link (parent) { - this.unlink() + const oldParent = this._parent + this._parent = parent this._parent.attach(this) + + oldParent && oldParent.detach(this) } unlink () { diff --git a/src/scope/context_execution.js b/src/scope/context_execution.js index 15ac27aae85..4246c81acc1 100644 --- a/src/scope/context_execution.js +++ b/src/scope/context_execution.js @@ -7,6 +7,7 @@ class ContextExecution { this._active = null this._count = 0 this._set = [] + this._executed = false if (context) { context.retain() @@ -43,14 +44,17 @@ class ContextExecution { this._set.splice(index, 1) this._active = this._set[this._set.length - 1] || null - } - exit () { - if (this._context && !this._active) { + if (this._executed) { this._bypass() } } + exit () { + this._executed = true + this._bypass() + } + attach (child) { this._children.add(child) this.retain() @@ -76,7 +80,9 @@ class ContextExecution { } _bypass () { - this._children.forEach(child => child.link(this._context.parent())) + if (this._context && !this._active) { + this._children.forEach(child => child.link(this._context.parent())) + } } } diff --git a/test/leak/scope_manager.js b/test/leak/scope_manager.js index 2cfab868663..7d0e92b5d2a 100644 --- a/test/leak/scope_manager.js +++ b/test/leak/scope_manager.js @@ -1,6 +1,6 @@ 'use strict' -require('../..').init() +const tracer = require('../..').init() const test = require('tape') const profile = require('../profile') @@ -12,3 +12,17 @@ test('ScopeManager should destroy executions even if their context is already de Promise.resolve().then(done) } }) + +test('ScopeManager should not leak when using scopes with recursive timers', t => { + profile(t, operation) + + function operation (done) { + const active = tracer.scopeManager().active() + + active && active.close() + + tracer.scopeManager().activate({}) + + done() + } +})