Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Reusable VM allows context leak due to global variable reassignment #1476

Open
mjameswh opened this issue Jul 24, 2024 · 0 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@mjameswh
Copy link
Contributor

Describe the bug

When the reuseV8Context option is enabled, we freeze global objects to prevent modifications from leaking from one Workflow execution context to another one.

However, we do not prevent reassignment of global variables. And since context switching is based solely on global variable names, the engine will leave those reassigned global variable untouched.

For example, the following test fails:

export async function sharedGlobalReassignment(): Promise<[string, string]> {
  type ConsoleExtended = Console & { wfid: string };
  globalThis.console = { ...console, wfid: workflowInfo().workflowId } as ConsoleExtended;
  await sleep(1);
  return [workflowInfo().workflowId, (console as ConsoleExtended).wfid];
}

test('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([
      env.client.workflow.execute(sharedGlobalReassignment, { taskQueue, workflowId: randomUUID() }),
      env.client.workflow.execute(sharedGlobalReassignment, { taskQueue, workflowId: randomUUID() }),
    ]);
    t.deepEqual(res1[0], res1[1]);
    t.deepEqual(res2[0], res2[1]);
  });
});

Deleting a global variable also fails, as the variable "reappears" on the next context switch. For example, the following test fails:

export async function globalMutatorAndDestructor(): Promise<number> {
  const global = globalThis as { a?: number };
  global.a = (global.a || 0) + 1;
  await sleep(1);
  delete global.a;
  await sleep(1);
  global.a = (global.a || 0) + 1;
  return global.a;
}

test('Set then Delete a global property', withReusableContext, async (t) => {
  const { createWorker, taskQueue, env } = t.context;
  const worker = await createWorker();
  await worker.runUntil(async () => {
    const res = await env.client.workflow.execute(globalMutatorAndDestructor, { taskQueue, workflowId: randomUUID() });
    t.is(res, 1);
  });
});
@mjameswh mjameswh added the bug Something isn't working label Jul 24, 2024
@mjameswh mjameswh self-assigned this Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant