Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions packages/cubejs-backend-shared/src/disposedProxy.ts
Original file line number Diff line number Diff line change
@@ -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.`
);
}
});
}
1 change: 1 addition & 0 deletions packages/cubejs-backend-shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export * from './platform';
export * from './FileRepository';
export * from './decorators';
export * from './PerfTracker';
export * from './disposedProxy';
107 changes: 107 additions & 0 deletions packages/cubejs-backend-shared/test/disposedProxy.test.ts
Original file line number Diff line number Diff line change
@@ -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/);
});
});
7 changes: 7 additions & 0 deletions packages/cubejs-server-core/src/core/CompilerApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -185,6 +186,12 @@ export class CompilerApi {
clearInterval(this.compiledScriptCacheInterval);
this.compiledScriptCacheInterval = null;
}

// 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;
}

public setGraphQLSchema(schema: GraphQLSchema): void {
Expand Down
109 changes: 109 additions & 0 deletions packages/cubejs-server-core/test/unit/CompilerApi.test.ts
Original file line number Diff line number Diff line change
@@ -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<Compiler> | 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/);
});
});
});
Loading