From d2b43cf67ae88b62e352ff107e1e5b98d5cf0125 Mon Sep 17 00:00:00 2001 From: remorses Date: Thu, 4 Jan 2024 18:28:23 +0100 Subject: [PATCH 1/2] use context for callId --- lib/sinon/proxy-invoke.js | 7 ++-- lib/sinon/proxy.js | 35 +++++++++---------- lib/sinon/sandbox.js | 9 +++-- lib/sinon/spy.js | 19 +++++++---- lib/sinon/util/core/global-context.js | 5 +++ test/sandbox-test.js | 48 +++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 30 deletions(-) create mode 100644 lib/sinon/util/core/global-context.js diff --git a/lib/sinon/proxy-invoke.js b/lib/sinon/proxy-invoke.js index 1b120ece5..6f96959c6 100644 --- a/lib/sinon/proxy-invoke.js +++ b/lib/sinon/proxy-invoke.js @@ -2,6 +2,7 @@ const arrayProto = require("@sinonjs/commons").prototypes.array; const proxyCallUtil = require("./proxy-call-util"); +const globalContext = require("./util/core/global-context"); const push = arrayProto.push; const forEach = arrayProto.forEach; @@ -9,11 +10,9 @@ const concat = arrayProto.concat; const ErrorConstructor = Error.prototype.constructor; const bind = Function.prototype.bind; -let callId = 0; - -module.exports = function invoke(func, thisValue, args) { +module.exports = function invoke(func, thisValue, args, ctx = globalContext) { const matchings = this.matchingFakes(args); - const currentCallId = callId++; + const currentCallId = ctx.callId++; let exception, returnValue; proxyCallUtil.incrementCallCount(this); diff --git a/lib/sinon/proxy.js b/lib/sinon/proxy.js index 47a137089..6197a6b19 100644 --- a/lib/sinon/proxy.js +++ b/lib/sinon/proxy.js @@ -2,6 +2,7 @@ const arrayProto = require("@sinonjs/commons").prototypes.array; const extend = require("./util/core/extend"); +const globalContext = require("./util/core/global-context"); const functionToString = require("./util/core/function-to-string"); const proxyCall = require("./proxy-call"); const proxyCallUtil = require("./proxy-call-util"); @@ -240,8 +241,8 @@ delegateToCalls(proxyApi, "alwaysReturned", false, "returned"); delegateToCalls(proxyApi, "calledWithNew", true); delegateToCalls(proxyApi, "alwaysCalledWithNew", false, "calledWithNew"); -function createProxy(func, originalFunc) { - const proxy = wrapFunction(func, originalFunc); +function createProxy(func, originalFunc, ctx = globalContext) { + const proxy = wrapFunction(func, originalFunc, ctx); // Inherit function properties: extend(proxy, func); @@ -253,7 +254,7 @@ function createProxy(func, originalFunc) { return proxy; } -function wrapFunction(func, originalFunc) { +function wrapFunction(func, originalFunc, ctx) { const arity = originalFunc.length; let p; // Do not change this to use an eval. Projects that depend on sinon block the use of eval. @@ -262,72 +263,72 @@ function wrapFunction(func, originalFunc) { /*eslint-disable no-unused-vars, max-len*/ case 0: p = function proxy() { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 1: p = function proxy(a) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 2: p = function proxy(a, b) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 3: p = function proxy(a, b, c) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 4: p = function proxy(a, b, c, d) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 5: p = function proxy(a, b, c, d, e) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 6: p = function proxy(a, b, c, d, e, f) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 7: p = function proxy(a, b, c, d, e, f, g) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 8: p = function proxy(a, b, c, d, e, f, g, h) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 9: p = function proxy(a, b, c, d, e, f, g, h, i) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 10: p = function proxy(a, b, c, d, e, f, g, h, i, j) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 11: p = function proxy(a, b, c, d, e, f, g, h, i, j, k) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; case 12: p = function proxy(a, b, c, d, e, f, g, h, i, j, k, l) { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; default: p = function proxy() { - return p.invoke(func, this, slice(arguments)); + return p.invoke(func, this, slice(arguments), ctx); }; break; /*eslint-enable*/ diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 6bab45f96..b5410153e 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -445,8 +445,13 @@ function Sandbox(opts = {}) { return spy; } - sandbox.spy = function spy() { - const createdSpy = sinonSpy.apply(sinonSpy, arguments); + sandbox.spy = function spy(object, property, types) { + const createdSpy = sinonSpy.apply(sinonSpy, [ + object, + property, + types, + sandbox, + ]); return commonPostInitSetup(arguments, createdSpy); }; diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index a10e26bf6..5eb11344c 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -11,6 +11,7 @@ const proxyCallUtil = require("./proxy-call-util"); const walkObject = require("./util/core/walk-object"); const wrapMethod = require("./util/core/wrap-method"); const valueToString = require("@sinonjs/commons").valueToString; +const globalContext = require("./util/core/global-context"); /* cache references to library methods so that they also can be stubbed without problems */ const forEach = arrayProto.forEach; @@ -130,7 +131,7 @@ delegateToCalls( }, ); -function createSpy(func) { +function createSpy(func, context = globalContext) { let name; let funk = func; @@ -142,7 +143,7 @@ function createSpy(func) { name = functionName(funk); } - const proxy = createProxy(funk, funk); + const proxy = createProxy(funk, funk, context); // Inherit spy API: extend.nonEnum(proxy, spyApi); @@ -155,13 +156,13 @@ function createSpy(func) { return proxy; } -function spy(object, property, types) { +function spy(object, property, types, context = globalContext) { if (isEsModule(object)) { throw new TypeError("ES Modules cannot be spied"); } if (!property && typeof object === "function") { - return createSpy(object); + return createSpy(object, context); } if (!property && typeof object === "object") { @@ -171,18 +172,22 @@ function spy(object, property, types) { if (!object && !property) { return createSpy(function () { return; - }); + }, context); } if (!types) { - return wrapMethod(object, property, createSpy(object[property])); + return wrapMethod( + object, + property, + createSpy(object[property], context), + ); } const descriptor = {}; const methodDesc = getPropertyDescriptor(object, property); forEach(types, function (type) { - descriptor[type] = createSpy(methodDesc[type]); + descriptor[type] = createSpy(methodDesc[type], context); }); return wrapMethod(object, property, descriptor); diff --git a/lib/sinon/util/core/global-context.js b/lib/sinon/util/core/global-context.js new file mode 100644 index 000000000..1d845ca1b --- /dev/null +++ b/lib/sinon/util/core/global-context.js @@ -0,0 +1,5 @@ +"use strict"; + +module.exports = { + callId: 0, +}; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index cc1818015..7f315aaf8 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -2377,4 +2377,52 @@ describe("Sandbox", function () { sandboxB.restore(); }); }); + describe("concurrency safe", function () { + it("spy", async function () { + class F { + constructor() { + this.first = []; + this.second = []; + this.third = []; + } + async run() { + await this.execute("first"); + await Promise.resolve(); + await this.execute("second"); + await Promise.resolve(); + await this.execute("third"); + } + + async execute(s) { + for (const f of this[s]) { + await f(); + } + } + } + + const fn = async () => { + const sinonSandbox = sinon.createSandbox(); + + const f = new F(); + + const a = sinonSandbox.spy(); + const b = sinonSandbox.spy(); + const c = sinonSandbox.spy(); + + f.first.push(a); + f.second.push(b); + f.third.push(c); + + await f.run(); + + assert(a.calledBefore(b)); + assert(b.calledBefore(c)); + + assert(a.calledImmediatelyBefore(b)); + assert(b.calledImmediatelyBefore(c)); + }; + + await Promise.all([fn, fn, fn]); + }); + }); }); From fabf95ca1f1cc06e996701722a4490d82797d68d Mon Sep 17 00:00:00 2001 From: remorses Date: Thu, 4 Jan 2024 18:48:23 +0100 Subject: [PATCH 2/2] add callId = 0 in sandbox --- lib/sinon/proxy-invoke.js | 3 +++ lib/sinon/proxy.js | 2 +- lib/sinon/sandbox.js | 1 + test/sandbox-test.js | 4 ++-- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/sinon/proxy-invoke.js b/lib/sinon/proxy-invoke.js index 6f96959c6..b81f9b0e9 100644 --- a/lib/sinon/proxy-invoke.js +++ b/lib/sinon/proxy-invoke.js @@ -12,6 +12,9 @@ const bind = Function.prototype.bind; module.exports = function invoke(func, thisValue, args, ctx = globalContext) { const matchings = this.matchingFakes(args); + if (ctx.callId == null) { + ctx.callId = 0; + } const currentCallId = ctx.callId++; let exception, returnValue; diff --git a/lib/sinon/proxy.js b/lib/sinon/proxy.js index 6197a6b19..330f2a0bb 100644 --- a/lib/sinon/proxy.js +++ b/lib/sinon/proxy.js @@ -254,7 +254,7 @@ function createProxy(func, originalFunc, ctx = globalContext) { return proxy; } -function wrapFunction(func, originalFunc, ctx) { +function wrapFunction(func, originalFunc, ctx = globalContext) { const arity = originalFunc.length; let p; // Do not change this to use an eval. Projects that depend on sinon block the use of eval. diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index b5410153e..64e63b707 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -78,6 +78,7 @@ function checkForValidArguments(descriptor, property, replacement) { */ function Sandbox(opts = {}) { const sandbox = this; + sandbox.callId = 0; const assertOptions = opts.assertOptions || {}; let fakeRestorers = []; let promiseLib; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 7f315aaf8..7d2f0ae76 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -2401,7 +2401,7 @@ describe("Sandbox", function () { } const fn = async () => { - const sinonSandbox = sinon.createSandbox(); + const sinonSandbox = createSandbox(); const f = new F(); @@ -2422,7 +2422,7 @@ describe("Sandbox", function () { assert(b.calledImmediatelyBefore(c)); }; - await Promise.all([fn, fn, fn]); + await Promise.all([fn(), fn(), fn(), fn()]); }); }); });