diff --git a/packages/common/src/type-helpers.ts b/packages/common/src/type-helpers.ts index e5d87f43f..869146cd6 100644 --- a/packages/common/src/type-helpers.ts +++ b/packages/common/src/type-helpers.ts @@ -219,8 +219,35 @@ export function SymbolBasedInstanceOfError(markerName: string): }; } -// Thanks MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze -export function deepFreeze(object: T): T { +// This implementation is based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze. +// Note that there are limits to this approach, as traversing using getOwnPropertyXxx doesn't allow +// reaching variables defined using internal scopes. This implementation specifically look for and deal +// with the case of the Map or Set classes, but it is impossible to cover all potential cases. +export function deepFreeze(object: T, visited = new WeakSet(), path = 'root'): T { + if (typeof object !== 'object' || object == null || Object.isFrozen(object) || visited.has(object)) return object; + visited.add(object); + + if (object.constructor?.name === 'Map' || object instanceof Map) { + const map = object as unknown as Map; + for (const [k, v] of map.entries()) { + deepFreeze(k, visited, `${path}[key:${k}]`); + deepFreeze(v, visited, `${path}[value:${k}]`); + } + map.set = frozenMapSet; + map.delete = frozenMapDelete; + map.clear = frozenMapClear; + } + + if (object.constructor?.name === 'Set' || object instanceof Set) { + const set = object as unknown as Set; + for (const v of set.values()) { + deepFreeze(v, visited, `${path}.${v}`); + } + set.add = frozenSetAdd; + set.delete = frozenSetDelete; + set.clear = frozenSetClear; + } + // Retrieve the property names defined on object const propNames = Object.getOwnPropertyNames(object); @@ -230,8 +257,11 @@ export function deepFreeze(object: T): T { if (value && typeof value === 'object') { try { - deepFreeze(value); - } catch (_err) { + deepFreeze(value, visited, `${path}.${name}`); + } catch (err) { + if (typeof value !== 'object' || value.constructor.name !== 'Uint8Array') { + console.log(`Failed to freeze ${path}.${name}`, err); + } // This is okay, there are some typed arrays that cannot be frozen (encodingKeys) } } else if (typeof value === 'function') { @@ -241,3 +271,27 @@ export function deepFreeze(object: T): T { return Object.freeze(object); } + +function frozenMapSet(key: any, _value: any): Map { + throw `Can't add property ${key}, map is not extensible`; +} + +function frozenMapDelete(key: any): boolean { + throw `Can't delete property ${key}, map is frozen`; +} + +function frozenMapClear() { + throw "Can't clear map, map is frozen"; +} + +function frozenSetAdd(key: any): Set { + throw `Can't add key ${key}, set is not extensible`; +} + +function frozenSetDelete(key: any): boolean { + throw `Can't delete key ${key}, set is not extensible`; +} + +function frozenSetClear() { + throw "Can't clear set, set is frozen"; +} diff --git a/packages/test/src/test-isolation.ts b/packages/test/src/test-isolation.ts index dd9a1425b..46357ff49 100644 --- a/packages/test/src/test-isolation.ts +++ b/packages/test/src/test-isolation.ts @@ -45,9 +45,8 @@ test.after.always(async (t) => { }); export async function globalMutator(): Promise { - const global = globalThis as { a?: number; b?: string }; + const global = globalThis as { a?: number }; global.a = (global.a || 0) + 1; - global.b = (global.b || '') + '/' + workflowInfo().workflowId; await sleep(1); global.a = (global.a || 0) + 1; return global.a; @@ -57,16 +56,8 @@ test('Global state is isolated and maintained between activations', async (t) => const { createWorker, taskQueue, env } = t.context; const worker = await createWorker(); await worker.runUntil(async () => { - const res1 = await env.client.workflow.execute(globalMutator, { - taskQueue, - workflowId: randomUUID(), - workflowTaskTimeout: '5m', - }); - const res2 = await env.client.workflow.execute(globalMutator, { - taskQueue, - workflowId: randomUUID(), - workflowTaskTimeout: '5m', - }); + const res1 = await env.client.workflow.execute(globalMutator, { taskQueue, workflowId: randomUUID() }); + const res2 = await env.client.workflow.execute(globalMutator, { taskQueue, workflowId: randomUUID() }); t.is(res1, 2); t.is(res2, 2); }); @@ -111,9 +102,33 @@ test('Module state is isolated and maintained between activations', async (t) => }); }); +const someMap: Map = new Map(); + +export async function moduleMapMutator(): Promise<[number, number]> { + someMap.set('test', (someMap.get('test') || 0) + 1); + const middle = someMap.get('test') as number; + await sleep(1); + someMap.set('test', (someMap.get('test') || 0) + 1); + const final = someMap.get('test') as number; + return [middle, final]; +} + +test('Deep Freeze reaches Map values', async (t) => { + const { createWorker, taskQueue, env } = t.context; + const worker = await createWorker(); + await worker.runUntil(async () => { + const [res1, res2] = await Promise.all([ + env.client.workflow.execute(moduleMapMutator, { taskQueue, workflowId: randomUUID() }), + env.client.workflow.execute(moduleMapMutator, { taskQueue, workflowId: randomUUID() }), + ]); + t.deepEqual(res1, [1, 2]); + t.deepEqual(res2, [1, 2]); + }); +}); + export async function sharedGlobalMutator(): Promise { try { - (setTimeout as any).a = 1; + (Array as any).a = 1; } catch (err) { throw ApplicationFailure.fromError(err); } @@ -131,9 +146,7 @@ test('Shared global state is frozen', withReusableContext, async (t) => { export async function sharedGlobalReassignment(): Promise<[string, string, string]> { type ConsoleExtended = Console & { wfid: string }; // Replace the `console` global by a new object - // eslint-disable-next-line no-debugger - debugger; - globalThis.console = { ...console, wfid: workflowInfo().workflowId } as ConsoleExtended; + globalThis.console = { ...globalThis.console, wfid: workflowInfo().workflowId } as ConsoleExtended; const middle = (globalThis.console as ConsoleExtended).wfid; await sleep(50); return [workflowInfo().workflowId, middle, (globalThis.console as ConsoleExtended).wfid]; @@ -143,14 +156,14 @@ test('Reassign shared global state', withReusableContext, async (t) => { const { createWorker, taskQueue, env } = t.context; const worker = await createWorker(); await worker.runUntil(async () => { - const [res1 /*, res2*/] = await Promise.all([ + const [res1, res2] = await Promise.all([ + env.client.workflow.execute(sharedGlobalReassignment, { taskQueue, workflowId: randomUUID() }), env.client.workflow.execute(sharedGlobalReassignment, { taskQueue, workflowId: randomUUID() }), - // env.client.workflow.execute(sharedGlobalReassignment, { taskQueue, workflowId: randomUUID() }), ]); t.deepEqual(res1[0], res1[1]); t.deepEqual(res1[0], res1[2]); - // t.deepEqual(res2[0], res2[1]); - // t.deepEqual(res2[0], res2[2]); + t.deepEqual(res2[0], res2[1]); + t.deepEqual(res2[0], res2[2]); }); }); diff --git a/packages/worker/src/workflow/reusable-vm.ts b/packages/worker/src/workflow/reusable-vm.ts index efcd10835..be623cfd9 100644 --- a/packages/worker/src/workflow/reusable-vm.ts +++ b/packages/worker/src/workflow/reusable-vm.ts @@ -84,24 +84,27 @@ export class ReusableVMWorkflowCreator implements WorkflowCreator { this.pristine = new Map(); (globals as any).__pristine = this.pristine; // The V8 context is really composed of two distinct objects: the `globals` object defined above - // on one side, and another internal object to which we don't have access from the JS side, which - // defines the built-in global properties. Those properties are not accessible from here + // on the outside, and another internal object to which we only have access from the inside, which + // defines the built-in global properties. We need vm.runInContext( ` const pristine = globalThis.__pristine; delete globalThis.__pristine; - for (const pname of Object.getOwnPropertyNames(globalThis)) { - const pdesc = Object.getOwnPropertyDescriptor(globalThis, pname); - pristine.set(pname, pdesc); + for (const name of Object.getOwnPropertyNames(globalThis)) { + const descriptor = Object.getOwnPropertyDescriptor(globalThis, name); + pristine.set(name, descriptor); } `, this.context ); - deepFreeze(this.pristine); - // deepFreeze(__webpack_module_cache__); - // for (const v of sharedModules.values()) { - // deepFreeze(v); - // } + for (const [k, v] of this.pristine.entries()) { + if (k !== 'globalThis') { + deepFreeze(v.value); + } + } + for (const v of sharedModules.values()) { + deepFreeze(v); + } } protected get context(): vm.Context { @@ -168,8 +171,11 @@ export class ReusableVMWorkflowCreator implements WorkflowCreator { context.__TEMPORAL_ARGS__ = keysToCleanup; vm.runInContext( ` - for (const [pname, pdesc] of globalThis.__TEMPORAL_ARGS__) { - delete globalThis[pname]; + for (const [name, descriptor] of globalThis.__TEMPORAL_ARGS__) { + delete globalThis[name]; + if (descriptor) { + Object.defineProperty(globalThis, name, descriptor); + } } delete globalThis.__TEMPORAL_ARGS__;`, context