From 0117069513c29ffb618aa07cc6056d9bf7c38b28 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Wed, 21 Apr 2021 10:32:46 -0700 Subject: [PATCH 1/4] Warn of potential memory leaks - Warn on the console if a potential memory leak is detected - Allow adjusting of the leak detection threshold --- docs/release-source/release/sandbox.md | 4 +++ lib/sinon/sandbox.js | 38 ++++++++++++++++------ test/sandbox-test.js | 45 ++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/docs/release-source/release/sandbox.md b/docs/release-source/release/sandbox.md index 74803acf7..8f346ff09 100644 --- a/docs/release-source/release/sandbox.md +++ b/docs/release-source/release/sandbox.md @@ -334,3 +334,7 @@ Verifies all mocks created through the sandbox. #### `sandbox.verifyAndRestore();` Verifies all mocks and restores all fakes created through the sandbox. + +#### `sandbox.leakThreshold` + +Gets/sets the threshold at which memory leak detection warnings are logged. diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 7b1c4c814..cf05d43c3 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -16,6 +16,8 @@ var fakeServer = require("nise").fakeServer; var fakeXhr = require("nise").fakeXhr; var usePromiseLibrary = require("./util/core/use-promise-library"); +var DEFAULT_LEAK_THRESHOLD = 1000; + var filter = arrayProto.filter; var forEach = arrayProto.forEach; var push = arrayProto.push; @@ -33,10 +35,26 @@ function applyOnEach(fakes, method) { function Sandbox() { var sandbox = this; - var collection = []; var fakeRestorers = []; var promiseLib; + var collection = []; + var loggedLeakWarning = false; + sandbox.leakThreshold = DEFAULT_LEAK_THRESHOLD; + + function track(object) { + if ( + push(collection, object) > sandbox.leakThreshold && + !loggedLeakWarning + ) { + // eslint-disable-next-line no-console + console.warn( + "Potential memory leak detected; be sure to call restore() to clean up your sandbox. To suppress this warning, modify the leakThreshold property of your sandbox." + ); + loggedLeakWarning = true; + } + } + sandbox.assert = sinonAssert.createAssertObject(); sandbox.serverPrototype = fakeServer; @@ -57,7 +75,7 @@ function Sandbox() { var ownMethods = collectOwnMethods(stubbed); forEach(ownMethods, function (method) { - push(collection, method); + track(method); }); usePromiseLibrary(promiseLib, ownMethods); @@ -115,7 +133,7 @@ function Sandbox() { sandbox.mock = function mock() { var m = sinonMock.apply(null, arguments); - push(collection, m); + track(m); usePromiseLibrary(promiseLib, m); return m; @@ -348,12 +366,12 @@ function Sandbox() { var ownMethods = collectOwnMethods(spy); forEach(ownMethods, function (method) { - push(collection, method); + track(method); }); usePromiseLibrary(promiseLib, ownMethods); } else { - push(collection, spy); + track(spy); usePromiseLibrary(promiseLib, spy); } @@ -374,7 +392,7 @@ function Sandbox() { sandbox.fake = function fake(f) { var s = sinonFake.apply(sinonFake, arguments); - push(collection, s); + track(s); return s; }; @@ -385,7 +403,7 @@ function Sandbox() { sandbox.fake[key] = function () { var s = fakeBehavior.apply(fakeBehavior, arguments); - push(collection, s); + track(s); return s; }; @@ -396,7 +414,7 @@ function Sandbox() { var clock = sinonClock.useFakeTimers.call(null, args); sandbox.clock = clock; - push(collection, clock); + track(clock); return clock; }; @@ -429,14 +447,14 @@ function Sandbox() { } sandbox.server = proto.create(); - push(collection, sandbox.server); + track(sandbox.server); return sandbox.server; }; sandbox.useFakeXMLHttpRequest = function useFakeXMLHttpRequest() { var xhr = fakeXhr.useFakeXMLHttpRequest(); - push(collection, xhr); + track(xhr); return xhr; }; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index f51caefc7..9530b9b8d 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -134,6 +134,51 @@ describe("Sandbox", function () { assert.equals(fakes.length, 2); }); + + describe("warns of potential leak when", function () { + var warn; + + beforeEach(function () { + warn = this.sandbox.stub(console, "warn"); + }); + + afterEach(function () { + warn.restore(); + }); + + it("many fakes are created", function () { + assert.equals(typeof this.sandbox.leakThreshold, "number"); + + createTooManyFakes(this.sandbox); + + assert(warn.called); + }); + + it("a configurable number of fakes are created", function () { + this.sandbox.leakThreshold = 20; + + createTooManyFakes(this.sandbox); + + assert(warn.called); + }); + + it("a leak warning has not already been output", function () { + this.sandbox.leakThreshold = 20; + + createTooManyFakes(this.sandbox); + this.sandbox.restore(); + warn.resetHistory(); + + createTooManyFakes(this.sandbox); + assert(!warn.called); + }); + + function createTooManyFakes(sandbox) { + for (var i = 0; i < sandbox.leakThreshold; i++) { + sandbox.spy(); + } + } + }); }); describe(".spy", function () { From 442b4d3c516a113c2d78b45af8270b4ae5f21492 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 22 Apr 2021 08:58:40 -0700 Subject: [PATCH 2/4] Use deprecated logger instead of console --- lib/sinon/sandbox.js | 3 ++- test/sandbox-test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index cf05d43c3..5d4d860c1 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -1,6 +1,7 @@ "use strict"; var arrayProto = require("@sinonjs/commons").prototypes.array; +var logger = require("@sinonjs/commons").deprecated; var collectOwnMethods = require("./collect-own-methods"); var getPropertyDescriptor = require("./util/core/get-property-descriptor"); var isPropertyConfigurable = require("./util/core/is-property-configurable"); @@ -48,7 +49,7 @@ function Sandbox() { !loggedLeakWarning ) { // eslint-disable-next-line no-console - console.warn( + logger.printWarning( "Potential memory leak detected; be sure to call restore() to clean up your sandbox. To suppress this warning, modify the leakThreshold property of your sandbox." ); loggedLeakWarning = true; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 9530b9b8d..419ecbfc5 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -139,7 +139,7 @@ describe("Sandbox", function () { var warn; beforeEach(function () { - warn = this.sandbox.stub(console, "warn"); + warn = this.sandbox.stub(deprecated, "printWarning"); }); afterEach(function () { From 717c73ba5defbadb9c45894a1e3399159848cc46 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 22 Apr 2021 09:00:23 -0700 Subject: [PATCH 3/4] Use 10,000 objects as memory leak threshold --- lib/sinon/sandbox.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 5d4d860c1..ca9bc3461 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -17,7 +17,7 @@ var fakeServer = require("nise").fakeServer; var fakeXhr = require("nise").fakeXhr; var usePromiseLibrary = require("./util/core/use-promise-library"); -var DEFAULT_LEAK_THRESHOLD = 1000; +var DEFAULT_LEAK_THRESHOLD = 10000; var filter = arrayProto.filter; var forEach = arrayProto.forEach; From 3038151231b2709aefbf57b92d10b6820cab0d26 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Thu, 22 Apr 2021 11:50:41 -0700 Subject: [PATCH 4/4] track -> addToCollection --- lib/sinon/sandbox.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index ca9bc3461..66fbe1c6f 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -43,7 +43,7 @@ function Sandbox() { var loggedLeakWarning = false; sandbox.leakThreshold = DEFAULT_LEAK_THRESHOLD; - function track(object) { + function addToCollection(object) { if ( push(collection, object) > sandbox.leakThreshold && !loggedLeakWarning @@ -76,7 +76,7 @@ function Sandbox() { var ownMethods = collectOwnMethods(stubbed); forEach(ownMethods, function (method) { - track(method); + addToCollection(method); }); usePromiseLibrary(promiseLib, ownMethods); @@ -134,7 +134,7 @@ function Sandbox() { sandbox.mock = function mock() { var m = sinonMock.apply(null, arguments); - track(m); + addToCollection(m); usePromiseLibrary(promiseLib, m); return m; @@ -367,12 +367,12 @@ function Sandbox() { var ownMethods = collectOwnMethods(spy); forEach(ownMethods, function (method) { - track(method); + addToCollection(method); }); usePromiseLibrary(promiseLib, ownMethods); } else { - track(spy); + addToCollection(spy); usePromiseLibrary(promiseLib, spy); } @@ -393,7 +393,7 @@ function Sandbox() { sandbox.fake = function fake(f) { var s = sinonFake.apply(sinonFake, arguments); - track(s); + addToCollection(s); return s; }; @@ -404,7 +404,7 @@ function Sandbox() { sandbox.fake[key] = function () { var s = fakeBehavior.apply(fakeBehavior, arguments); - track(s); + addToCollection(s); return s; }; @@ -415,7 +415,7 @@ function Sandbox() { var clock = sinonClock.useFakeTimers.call(null, args); sandbox.clock = clock; - track(clock); + addToCollection(clock); return clock; }; @@ -448,14 +448,14 @@ function Sandbox() { } sandbox.server = proto.create(); - track(sandbox.server); + addToCollection(sandbox.server); return sandbox.server; }; sandbox.useFakeXMLHttpRequest = function useFakeXMLHttpRequest() { var xhr = fakeXhr.useFakeXMLHttpRequest(); - track(xhr); + addToCollection(xhr); return xhr; };