From bba1fdcb7c8993fd8795dc126a10ea54cd3fa1b5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 10 May 2022 16:37:24 +0100 Subject: [PATCH] Switching over to using our own version of cls-hooked which has the memory leak (no async hooks disable call) fixed as well as changing how we use the CLS namespaces to allow us to destroy the namespace we use per request. --- packages/backend-core/package.json | 2 +- .../src/context/FunctionContext.js | 91 ++++++------------- packages/backend-core/src/context/index.js | 16 ++-- .../backend-core/src/middleware/tenancy.js | 28 +++--- packages/backend-core/yarn.lock | 2 +- packages/server/package.json | 1 + 6 files changed, 53 insertions(+), 87 deletions(-) diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index 4cccf2334..477a8ffe7 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -13,7 +13,7 @@ "@techpass/passport-openidconnect": "^0.3.0", "aws-sdk": "^2.901.0", "bcryptjs": "^2.4.3", - "cls-hooked": "^4.2.2", + "emitter-listener": "^1.1.2", "ioredis": "^4.27.1", "jsonwebtoken": "^8.5.1", "koa-passport": "^4.1.4", diff --git a/packages/backend-core/src/context/FunctionContext.js b/packages/backend-core/src/context/FunctionContext.js index 34d39492f..c0ed34fe7 100644 --- a/packages/backend-core/src/context/FunctionContext.js +++ b/packages/backend-core/src/context/FunctionContext.js @@ -1,84 +1,47 @@ -const cls = require("cls-hooked") +const cls = require("../clshooked") const { newid } = require("../hashing") const REQUEST_ID_KEY = "requestId" - -class FunctionContext { - static getMiddleware( - updateCtxFn = null, - destroyFn = null, - contextName = "session" - ) { - const namespace = this.createNamespace(contextName) - - return async function (ctx, next) { - await new Promise( - namespace.bind(function (resolve, reject) { - // store a contextual request ID that can be used anywhere (audit logs) - namespace.set(REQUEST_ID_KEY, newid()) - namespace.bindEmitter(ctx.req) - namespace.bindEmitter(ctx.res) - - if (updateCtxFn) { - updateCtxFn(ctx) - } - next() - .then(resolve) - .catch(reject) - .finally(() => { - if (destroyFn) { - return destroyFn(ctx) - } - }) - }) - ) - } +const MAIN_CTX = cls.createNamespace("main") + +function getContextStorage(namespace) { + if (namespace && namespace.active) { + let contextData = namespace.active + delete contextData.id + delete contextData._ns_name + return contextData } + return {} +} - static run(callback, contextName = "session") { - const namespace = this.createNamespace(contextName) - - return namespace.runAndReturn(callback) +class FunctionContext { + static run(callback) { + return MAIN_CTX.runAndReturn(async () => { + const namespaceId = newid() + MAIN_CTX.set(REQUEST_ID_KEY, namespaceId) + const namespace = cls.createNamespace(namespaceId) + let response = await namespace.runAndReturn(callback) + cls.destroyNamespace(namespaceId) + return response + }) } - static setOnContext(key, value, contextName = "session") { - const namespace = this.createNamespace(contextName) + static setOnContext(key, value) { + const namespaceId = MAIN_CTX.get(REQUEST_ID_KEY) + const namespace = cls.getNamespace(namespaceId) namespace.set(key, value) } - static getContextStorage() { - if (this._namespace && this._namespace.active) { - let contextData = this._namespace.active - delete contextData.id - delete contextData._ns_name - return contextData - } - - return {} - } - static getFromContext(key) { - const context = this.getContextStorage() + const namespaceId = MAIN_CTX.get(REQUEST_ID_KEY) + const namespace = cls.getNamespace(namespaceId) + const context = getContextStorage(namespace) if (context) { return context[key] } else { return null } } - - static destroyNamespace(name = "session") { - if (this._namespace) { - cls.destroyNamespace(name) - this._namespace = null - } - } - - static createNamespace(name = "session") { - if (!this._namespace) { - this._namespace = cls.createNamespace(name) - } - return this._namespace - } } module.exports = FunctionContext diff --git a/packages/backend-core/src/context/index.js b/packages/backend-core/src/context/index.js index b6b6f2380..20e5e2669 100644 --- a/packages/backend-core/src/context/index.js +++ b/packages/backend-core/src/context/index.js @@ -55,6 +55,15 @@ async function closeAppDBs() { } } +exports.closeTenancy = async () => { + if (env.USE_COUCH) { + await closeDB(exports.getGlobalDB()) + } + // clear from context now that database is closed/task is finished + cls.setOnContext(ContextKeys.TENANT_ID, null) + cls.setOnContext(ContextKeys.GLOBAL_DB, null) +} + exports.isDefaultTenant = () => { return exports.getTenantId() === exports.DEFAULT_TENANT_ID } @@ -82,12 +91,7 @@ exports.doInTenant = (tenantId, task) => { } finally { const using = cls.getFromContext(ContextKeys.IN_USE) if (!using || using <= 1) { - if (env.USE_COUCH) { - await closeDB(exports.getGlobalDB()) - } - // clear from context now that database is closed/task is finished - cls.setOnContext(ContextKeys.TENANT_ID, null) - cls.setOnContext(ContextKeys.GLOBAL_DB, null) + await exports.closeTenancy() } else { cls.setOnContext(using - 1) } diff --git a/packages/backend-core/src/middleware/tenancy.js b/packages/backend-core/src/middleware/tenancy.js index f4053d1f5..9a0cb8a0c 100644 --- a/packages/backend-core/src/middleware/tenancy.js +++ b/packages/backend-core/src/middleware/tenancy.js @@ -1,6 +1,5 @@ -const { setTenantId, setGlobalDB, getGlobalDB } = require("../tenancy") -const { closeDB } = require("../db") -const ContextFactory = require("../context/FunctionContext") +const { setTenantId, setGlobalDB, closeTenancy } = require("../tenancy") +const cls = require("../context/FunctionContext") const { buildMatcherRegex, matches } = require("./matchers") module.exports = ( @@ -11,17 +10,16 @@ module.exports = ( const allowQsOptions = buildMatcherRegex(allowQueryStringPatterns) const noTenancyOptions = buildMatcherRegex(noTenancyPatterns) - const updateCtxFn = ctx => { - const allowNoTenant = - opts.noTenancyRequired || !!matches(ctx, noTenancyOptions) - const allowQs = !!matches(ctx, allowQsOptions) - const tenantId = setTenantId(ctx, { allowQs, allowNoTenant }) - setGlobalDB(tenantId) + return async function (ctx, next) { + return cls.run(async () => { + const allowNoTenant = + opts.noTenancyRequired || !!matches(ctx, noTenancyOptions) + const allowQs = !!matches(ctx, allowQsOptions) + const tenantId = setTenantId(ctx, { allowQs, allowNoTenant }) + setGlobalDB(tenantId) + const res = await next() + await closeTenancy() + return res + }) } - const destroyFn = async () => { - const db = getGlobalDB() - await closeDB(db) - } - - return ContextFactory.getMiddleware(updateCtxFn, destroyFn) } diff --git a/packages/backend-core/yarn.lock b/packages/backend-core/yarn.lock index 87db3761b..fff682df5 100644 --- a/packages/backend-core/yarn.lock +++ b/packages/backend-core/yarn.lock @@ -1533,7 +1533,7 @@ electron-to-chromium@^1.3.896: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.900.tgz#5be2c5818a2a012c511b4b43e87b6ab7a296d4f5" integrity sha512-SuXbQD8D4EjsaBaJJxySHbC+zq8JrFfxtb4GIr4E9n1BcROyMcRrJCYQNpJ9N+Wjf5mFp7Wp0OHykd14JNEzzQ== -emitter-listener@^1.0.1: +emitter-listener@^1.0.1, emitter-listener@^1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/emitter-listener/-/emitter-listener-1.1.2.tgz#56b140e8f6992375b3d7cb2cab1cc7432d9632e8" integrity sha512-Bt1sBAGFHY9DKY+4/2cV6izcKJUf5T7/gkdmkxzX/qv9CcGH8xSwVRW5mtX03SWJtRTWSOpzCuWN9rBFYZepZQ== diff --git a/packages/server/package.json b/packages/server/package.json index f96568a80..46dc46547 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -10,6 +10,7 @@ }, "scripts": { "build": "rimraf dist/ && tsc -p tsconfig.build.json && mv dist/src/* dist/ && rimraf dist/src/ && yarn postbuild", + "debug": "yarn build && node --expose-gc --inspect=9222 dist/index.js", "postbuild": "copyfiles -u 1 src/**/*.svelte dist/ && copyfiles -u 1 src/**/*.hbs dist/ && copyfiles -u 1 src/**/*.json dist/", "test": "jest --coverage --maxWorkers=2", "test:watch": "jest --watch",