Skip to content

Commit

Permalink
Fix remaining issues
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjameswh
mjameswh committed Nov 22, 2024
1 parent fc22b2e commit 2a36b38
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 36 deletions.
62 changes: 58 additions & 4 deletions packages/common/src/type-helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,35 @@ export function SymbolBasedInstanceOfError<E extends Error>(markerName: string):
};
}

// Thanks MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze
export function deepFreeze<T>(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<T>(object: T, visited = new WeakSet<any>(), 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<any, any>;
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<any>;
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);

Expand All @@ -230,8 +257,11 @@ export function deepFreeze<T>(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') {
Expand All @@ -241,3 +271,27 @@ export function deepFreeze<T>(object: T): T {

return Object.freeze(object);
}

function frozenMapSet(key: any, _value: any): Map<any, any> {
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<any> {
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";
}
53 changes: 33 additions & 20 deletions packages/test/src/test-isolation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ test.after.always(async (t) => {
});

export async function globalMutator(): Promise<number> {
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;
Expand All @@ -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);
});
Expand Down Expand Up @@ -111,9 +102,33 @@ test('Module state is isolated and maintained between activations', async (t) =>
});
});

const someMap: Map<string, number> = 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<void> {
try {
(setTimeout as any).a = 1;
(Array as any).a = 1;
} catch (err) {
throw ApplicationFailure.fromError(err);
}
Expand All @@ -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];
Expand All @@ -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]);
});
});

Expand Down
30 changes: 18 additions & 12 deletions packages/worker/src/workflow/reusable-vm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,27 @@ export class ReusableVMWorkflowCreator implements WorkflowCreator {
this.pristine = new Map<string, PropertyDescriptor>();
(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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2a36b38

Please sign in to comment.