From 4c5e4a9af28481258d1e45537d4baad5a294c6a5 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Wed, 19 Nov 2025 11:23:25 +0200 Subject: [PATCH 1/3] perf(schema-compiler): More aggressive compilers cleanup --- packages/cubejs-server-core/src/core/CompilerApi.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cubejs-server-core/src/core/CompilerApi.ts b/packages/cubejs-server-core/src/core/CompilerApi.ts index e5a565ea58adc..01ebc5dfe4a64 100644 --- a/packages/cubejs-server-core/src/core/CompilerApi.ts +++ b/packages/cubejs-server-core/src/core/CompilerApi.ts @@ -185,6 +185,10 @@ export class CompilerApi { clearInterval(this.compiledScriptCacheInterval); this.compiledScriptCacheInterval = null; } + + this.compilers = undefined; + this.queryFactory = undefined; + this.graphqlSchema = undefined; } public setGraphQLSchema(schema: GraphQLSchema): void { From f19d376758b9d7d61697a823be8511d9c9dd5e7b Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 21 Nov 2025 16:09:15 +0200 Subject: [PATCH 2/3] add safeguards for cleaning instances --- .../src/disposedProxy.ts | 44 +++++++ packages/cubejs-backend-shared/src/index.ts | 1 + .../test/disposedProxy.test.ts | 107 ++++++++++++++++++ .../src/core/CompilerApi.ts | 7 +- 4 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 packages/cubejs-backend-shared/src/disposedProxy.ts create mode 100644 packages/cubejs-backend-shared/test/disposedProxy.test.ts diff --git a/packages/cubejs-backend-shared/src/disposedProxy.ts b/packages/cubejs-backend-shared/src/disposedProxy.ts new file mode 100644 index 0000000000000..bcb58fad0340c --- /dev/null +++ b/packages/cubejs-backend-shared/src/disposedProxy.ts @@ -0,0 +1,44 @@ +/** + * Creates a proxy object that throws an error on any property access. + * Used as a safeguard after disposal to catch dangling references. + */ +export function disposedProxy(name: string, instanceName: string): any { + return new Proxy({}, { + get(_target: object, prop: string | symbol): never { + throw new Error( + `Cannot access property '${String(prop)}' on ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + }, + set(_target: object, prop: string | symbol): never { + throw new Error( + `Cannot set property '${String(prop)}' on ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + }, + apply(): never { + throw new Error( + `Cannot call method on ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + }, + has(_target: object, _prop: string | symbol): never { + throw new Error( + `Cannot check property existence on ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + }, + ownKeys(): never { + throw new Error( + `Cannot enumerate properties on ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + }, + getPrototypeOf(): never { + throw new Error( + `Cannot get prototype of ${instanceName}. ` + + `The '${name}' has been cleaned up and is no longer available.` + ); + } + }); +} diff --git a/packages/cubejs-backend-shared/src/index.ts b/packages/cubejs-backend-shared/src/index.ts index 29f89efa7d72e..bcdb0a0b4414e 100644 --- a/packages/cubejs-backend-shared/src/index.ts +++ b/packages/cubejs-backend-shared/src/index.ts @@ -23,3 +23,4 @@ export * from './platform'; export * from './FileRepository'; export * from './decorators'; export * from './PerfTracker'; +export * from './disposedProxy'; diff --git a/packages/cubejs-backend-shared/test/disposedProxy.test.ts b/packages/cubejs-backend-shared/test/disposedProxy.test.ts new file mode 100644 index 0000000000000..3810bb3a11983 --- /dev/null +++ b/packages/cubejs-backend-shared/test/disposedProxy.test.ts @@ -0,0 +1,107 @@ +import { disposedProxy } from '../src'; + +describe('disposedProxy', () => { + test('should throw on property access', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => proxy.someProperty).toThrow( + "Cannot access property 'someProperty' on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on property set', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => { proxy.someProperty = 'value'; }).toThrow( + "Cannot set property 'someProperty' on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on method call', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => proxy.someMethod()).toThrow( + "Cannot access property 'someMethod' on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on nested function call', () => { + const proxy = disposedProxy('testFunction', 'test instance'); + + // Accessing a method on the proxy will throw, which is the expected behavior + // Note: Direct proxy() call actually triggers 'get' for '', not 'apply' + expect(() => proxy.someMethod()).toThrow( + "Cannot access property 'someMethod' on test instance. " + + "The 'testFunction' has been cleaned up and is no longer available." + ); + }); + + test('should throw on "in" operator', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => 'someProperty' in proxy).toThrow( + "Cannot check property existence on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on Object.keys', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => Object.keys(proxy)).toThrow( + "Cannot enumerate properties on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on Object.getPrototypeOf', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + + expect(() => Object.getPrototypeOf(proxy)).toThrow( + "Cannot get prototype of test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should include correct property name in error message', () => { + const proxy = disposedProxy('compilers', 'disposed CompilerApi instance'); + + expect(() => proxy.cubeEvaluator).toThrow( + "Cannot access property 'cubeEvaluator' on disposed CompilerApi instance. " + + "The 'compilers' has been cleaned up and is no longer available." + ); + }); + + test('should work with symbol properties', () => { + const proxy = disposedProxy('testProperty', 'test instance'); + const sym = Symbol('testSymbol'); + + expect(() => proxy[sym]).toThrow( + "Cannot access property 'Symbol(testSymbol)' on test instance. " + + "The 'testProperty' has been cleaned up and is no longer available." + ); + }); + + test('should throw on nested property access', () => { + const proxy = disposedProxy('queryFactory', 'disposed CompilerApi instance'); + + // First access throws + expect(() => proxy.createQuery).toThrow( + "Cannot access property 'createQuery' on disposed CompilerApi instance. " + + "The 'queryFactory' has been cleaned up and is no longer available." + ); + }); + + test('should provide helpful error message for real-world scenario', () => { + // Simulate the CompilerApi scenario + const compilers = disposedProxy('compilers', 'disposed CompilerApi instance'); + const queryFactory = disposedProxy('queryFactory', 'disposed CompilerApi instance'); + + // Both should throw helpful errors + expect(() => compilers.cubeEvaluator).toThrow(/disposed CompilerApi instance/); + expect(() => queryFactory.createQuery).toThrow(/disposed CompilerApi instance/); + }); +}); diff --git a/packages/cubejs-server-core/src/core/CompilerApi.ts b/packages/cubejs-server-core/src/core/CompilerApi.ts index 01ebc5dfe4a64..d0c863cbdb5ef 100644 --- a/packages/cubejs-server-core/src/core/CompilerApi.ts +++ b/packages/cubejs-server-core/src/core/CompilerApi.ts @@ -23,6 +23,7 @@ import { GraphQLSchema } from 'graphql'; import { parse as uuidParse, v4 as uuidv4 } from 'uuid'; import { LRUCache } from 'lru-cache'; import { NativeInstance } from '@cubejs-backend/native'; +import { disposedProxy } from '@cubejs-backend/shared'; import type { SchemaFileRepository } from '@cubejs-backend/shared'; import { NormalizedQuery, MemberExpression } from '@cubejs-backend/api-gateway'; import { DbTypeAsyncFn, DialectClassFn, LoggerFn } from './types'; @@ -186,8 +187,10 @@ export class CompilerApi { this.compiledScriptCacheInterval = null; } - this.compilers = undefined; - this.queryFactory = undefined; + // freeing memory-heavy allocated instances + // using safeguard for potential dangling references. + this.compilers = disposedProxy('compilers', 'disposed CompilerApi instance'); + this.queryFactory = disposedProxy('queryFactory', 'disposed CompilerApi instance'); this.graphqlSchema = undefined; } From 0b93c33584b4c9f0a7320b0a2afa8f982f656a7c Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 24 Nov 2025 11:51:28 +0200 Subject: [PATCH 3/3] add tests for compilerApi expose() --- .../test/unit/CompilerApi.test.ts | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 packages/cubejs-server-core/test/unit/CompilerApi.test.ts diff --git a/packages/cubejs-server-core/test/unit/CompilerApi.test.ts b/packages/cubejs-server-core/test/unit/CompilerApi.test.ts new file mode 100644 index 0000000000000..32c92dfe27a87 --- /dev/null +++ b/packages/cubejs-server-core/test/unit/CompilerApi.test.ts @@ -0,0 +1,109 @@ +import { SchemaFileRepository } from '@cubejs-backend/shared'; +import type { Compiler, QueryFactory } from '@cubejs-backend/schema-compiler'; +import { CompilerApi } from '../../src/core/CompilerApi'; +import { DbTypeAsyncFn } from '../../src/core/types'; + +// Test helper class to expose protected properties +class CompilerApiTestable extends CompilerApi { + public getCompilersProperty(): Promise | any { + return this.compilers; + } + + public getQueryFactoryProperty(): QueryFactory | any { + return this.queryFactory; + } +} + +describe('CompilerApi', () => { + describe('dispose', () => { + let compilerApi: CompilerApiTestable; + + // Mock repository + const mockRepository: SchemaFileRepository = { + localPath: () => '/mock/path', + dataSchemaFiles: () => Promise.resolve([ + { + fileName: 'test.js', + content: ` + cube('TestCube', { + sql: 'SELECT * FROM test', + measures: { + count: { + type: 'count' + } + } + }); + ` + } + ]) + }; + + // Mock dbType function + const mockDbType: DbTypeAsyncFn = async () => 'postgres'; + + beforeEach(() => { + compilerApi = new CompilerApiTestable( + mockRepository, + mockDbType, + { + logger: () => {}, // eslint-disable-line @typescript-eslint/no-empty-function + } + ); + }); + + afterEach(() => { + if (compilerApi) { + compilerApi.dispose(); + } + }); + + test('should replace compilers with disposed proxy after dispose', async () => { + await compilerApi.getCompilers(); + + compilerApi.dispose(); + + // Try to access compilers after dispose - should throw + const compilers = compilerApi.getCompilersProperty(); + + // Since compilers is now a disposed proxy (not a Promise), + // any property access should throw immediately + expect(() => compilers.cubeEvaluator).toThrow(/disposed CompilerApi instance/); + }); + + test('should replace queryFactory with disposed proxy after dispose', async () => { + await compilerApi.getCompilers(); + + compilerApi.dispose(); + + // Try to access queryFactory - should throw + const queryFactory = compilerApi.getQueryFactoryProperty(); + + expect(() => queryFactory.createQuery).toThrow(/disposed CompilerApi instance/); + }); + + test('should set graphqlSchema to undefined on dispose', async () => { + const mockSchema = {} as any; + compilerApi.setGraphQLSchema(mockSchema); + + expect(compilerApi.getGraphQLSchema()).toBe(mockSchema); + + compilerApi.dispose(); + + // Schema should be undefined + expect(compilerApi.getGraphQLSchema()).toBeUndefined(); + }); + + test('should be safe to call dispose multiple times', async () => { + await compilerApi.getCompilers(); + + compilerApi.dispose(); + compilerApi.dispose(); + compilerApi.dispose(); + + // Should still throw on access + const compilers = compilerApi.getCompilersProperty(); + + expect(() => compilers.cubeEvaluator).toThrow(/disposed CompilerApi instance/); + }); + }); +});