History

fix: tighten plugin callback boundaries

t3-code[bot] committed 2 months ago

Commit e3039b21 parent 7221a723 files · +139 −35Browse files

Unified · Split

3 files
  1. packages/plugin-runtime-poc/README.md +1 −1
  2. packages/plugin-runtime-poc/src/cordisRuntime.ts +53 −25
  3. packages/plugin-runtime-poc/src/runtimeContract.ts +85 −9
packages/plugin-runtime-poc/README.md+1−1
@@ -14,7 +14,7 @@ cordis satisfies the contract, but the adapter still needs a t3-specific planner · 13 unmodified lines
1414
1515 ## proved
1616
17−- 26 tests cover dependency order, blocking, rollback, cycles, unchanged fiber reuse, order-insensitive declarations, activation implementation replacement, active callback reentrancy, descendant async tasks, observer isolation, cleanup reporting, unusual ids and slots, and immutable snapshots
17+- 29 tests cover dependency order, blocking, rollback, cycles, unchanged fiber reuse, order-insensitive declarations, activation implementation replacement, active callback reentrancy, synchronous callback microtasks, descendant async tasks, late activation-context calls, queued input snapshots, observer isolation, cleanup reporting, unusual ids and slots, and immutable snapshots
1818 - manifest validation enforces semver, prevents entrypoint path escape, and keeps mobile declarative
1919 - the demo exercises activation, replacement, teardown, and a 250-plugin chain
2020
packages/plugin-runtime-poc/src/cordisRuntime.ts+53−25
@@ -332,9 +332,20 @@ export const createCordisRuntime: PluginRuntimeFactory = (options = {}): PluginR · 331 unmodified lines
332332 ): Promise<Result> => {
333333 const callbackState = { active: true };
334334 try {
335− return await callbackContext.run(callbackState, () => Promise.resolve().then(invoke));
336− } finally {
335+ const result = callbackContext.run(callbackState, invoke);
336+ if (
337+ (typeof result === "object" && result !== null && "then" in result) ||
338+ typeof result === "function"
339+ ) {
340+ return await Promise.resolve(result).finally(() => {
341+ callbackState.active = false;
342+ });
343+ }
344+ callbackState.active = false;
345+ return result;
346+ } catch (error) {
337347 callbackState.active = false;
348+ throw error;
338349 }
339350 };
340351
@@ -372,13 +383,14 @@ export const createCordisRuntime: PluginRuntimeFactory = (options = {}): PluginR · 31 unmodified lines
372383 }
373384 };
374385
375− const reconcile = (definitions: ReadonlyArray<PluginDefinition>) =>
376− runExclusive(async () => {
386+ const reconcile = (definitions: ReadonlyArray<PluginDefinition>) => {
387+ const desired = [...definitions];
388+ return runExclusive(async () => {
377389 if (disposed) throw new Error("plugin runtime is disposed");
378390
379391 // Validation and cycle detection happen before any candidate fibers are created,
380392 // so an invalid graph cannot touch the currently active composition.
381− const { blocked, ordered, providerByCapability } = analyzeDefinitions(definitions);
393+ const { blocked, ordered, providerByCapability } = analyzeDefinitions(desired);
382394 const affected = affectedPluginIds(composition.plugins, ordered);
383395 const currentById = new Map(
384396 composition.plugins.map((plugin) => [plugin.definition.id, plugin]),
@@ -419,6 +431,12 @@ export const createCordisRuntime: PluginRuntimeFactory = (options = {}): PluginR · 34 unmodified lines
419431
420432 const cordisPlugin: CordisPlugin = async (fiberContext) => {
421433 const finalizers: Array<() => void | Promise<void>> = [];
434+ let activating = true;
435+ const assertActivating = () => {
436+ if (!activating) {
437+ throw new Error(`activation context for ${definition.id} is no longer active`);
438+ }
439+ };
422440 fiberContext.effect(
423441 () => async () => {
424442 for (const finalizer of finalizers.toReversed()) {
@@ -436,26 +454,35 @@ export const createCordisRuntime: PluginRuntimeFactory = (options = {}): PluginR · 11 unmodified lines
436454 fiberContext.provide(capability, service);
437455 }
438456
439− await invokePluginCallback(() =>
440− definition.activate({
441− resolve: <Service>(capability: string) => fiberContext.get(capability) as Service,
442− register(slot, contribution) {
443− fiberContext.effect(() => {
444− const items = contributions.get(slot) ?? [];
445− items.push(contribution);
446− contributions.set(slot, items);
447− return () => {
448− const index = items.indexOf(contribution);
449− if (index >= 0) items.splice(index, 1);
450− if (!items.length) contributions.delete(slot);
451− };
452− }, `${definition.id}:contribution:${slot}`);
453− },
454− onDispose(finalizer) {
455− finalizers.push(finalizer);
456− },
457− }),
458− );
457+ try {
458+ await invokePluginCallback(() =>
459+ definition.activate({
460+ resolve: <Service>(capability: string) => {
461+ assertActivating();
462+ return fiberContext.get(capability) as Service;
463+ },
464+ register(slot, contribution) {
465+ assertActivating();
466+ fiberContext.effect(() => {
467+ const items = contributions.get(slot) ?? [];
468+ items.push(contribution);
469+ contributions.set(slot, items);
470+ return () => {
471+ const index = items.indexOf(contribution);
472+ if (index >= 0) items.splice(index, 1);
473+ if (!items.length) contributions.delete(slot);
474+ };
475+ }, `${definition.id}:contribution:${slot}`);
476+ },
477+ onDispose(finalizer) {
478+ assertActivating();
479+ finalizers.push(finalizer);
480+ },
481+ }),
482+ );
483+ } finally {
484+ activating = false;
485+ }
459486 };
460487 cordisPlugin.inject = [...(definition.requires ?? [])];
461488 cordisPlugin.provide = Object.keys(definition.provides ?? {});
@@ -502,6 +529,7 @@ export const createCordisRuntime: PluginRuntimeFactory = (options = {}): PluginR · 40 unmodified lines
502529 reportCleanupErrors("retire", cleanupErrors);
503530 return candidate.snapshot;
504531 });
532+ };
505533
506534 return {
507535 reconcile,
packages/plugin-runtime-poc/src/runtimeContract.ts+85−9
@@ -7,10 +7,20 @@ import type { PluginDefinition, PluginRuntimeFactory, PluginRuntimeSnapshot } fr · 6 unmodified lines
77 const contributionLabels = (snapshot: PluginRuntimeSnapshot, slot: string) =>
88 snapshot.contributions[slot]?.map((item) => item.label) ?? [];
99
10−const operationTimeout = () =>
11− NodeTimersPromises.setTimeout(100).then(() => {
10+const withOperationTimeout = async <Result>(operation: Promise<Result>): Promise<Result> => {
11+ const controller = new AbortController();
12+ const timeout = NodeTimersPromises.setTimeout(100, undefined, {
13+ ref: false,
14+ signal: controller.signal,
15+ }).then(() => {
1216 throw new Error("operation timed out");
1317 });
18+ try {
19+ return await Promise.race([operation, timeout]);
20+ } finally {
21+ controller.abort();
22+ }
23+};
1424
1525 const failureCause = (error: unknown): unknown =>
1626 typeof error === "object" && error !== null && "cause" in error
@@ -482,9 +492,7 @@ export function defineRuntimeContract(name: string, createRuntime: PluginRuntime · 465 unmodified lines
482492 await runtime.reconcile([]);
483493 },
484494 };
485− const timeout = operationTimeout();
486−
487− await expect(Promise.race([runtime.reconcile([reentrant]), timeout])).rejects.toThrow(
495+ await expect(withOperationTimeout(runtime.reconcile([reentrant]))).rejects.toThrow(
488496 "reentrant",
489497 );
490498 });
@@ -507,9 +515,7 @@ export function defineRuntimeContract(name: string, createRuntime: PluginRuntime · 16 unmodified lines
507515 },
508516 },
509517 ]);
510− const timeout = operationTimeout();
511−
512− await expect(Promise.race([runtime.reconcile([]), timeout])).resolves.toBeDefined();
518+ await expect(withOperationTimeout(runtime.reconcile([]))).resolves.toBeDefined();
513519 expect(nestedFailure).toBeInstanceOf(Error);
514520 expect((nestedFailure as Error).message).toContain("reentrant");
515521 await runtime.dispose();
@@ -534,7 +540,77 @@ export function defineRuntimeContract(name: string, createRuntime: PluginRuntime · 18 unmodified lines
534540 ]);
535541
536542 releaseBackground();
537− await expect(Promise.race([backgroundResult!, operationTimeout()])).resolves.toBeDefined();
543+ await expect(withOperationTimeout(backgroundResult!)).resolves.toBeDefined();
544+ await runtime.dispose();
545+ });
546+
547+ it("allows microtasks spawned by synchronous plugin callbacks to use the runtime", async () => {
548+ let backgroundResult: Promise<PluginRuntimeSnapshot> | undefined;
549+ const runtime = createRuntime();
550+
551+ await runtime.reconcile([
552+ {
553+ id: "acme.microtask",
554+ version: "1.0.0",
555+ activate() {
556+ queueMicrotask(() => {
557+ backgroundResult = runtime.reconcile([]);
558+ });
559+ },
560+ },
561+ ]);
562+
563+ expect(backgroundResult).toBeDefined();
564+ await expect(withOperationTimeout(backgroundResult!)).resolves.toBeDefined();
565+ await runtime.dispose();
566+ });
567+
568+ it("rejects finalizers registered after activation settles", async () => {
569+ let registerLateFinalizer!: () => void;
570+ let disposed = false;
571+ const runtime = createRuntime();
572+ await runtime.reconcile([
573+ {
574+ id: "acme.late-finalizer",
575+ version: "1.0.0",
576+ activate(context) {
577+ registerLateFinalizer = () =>
578+ context.onDispose(() => {
579+ disposed = true;
580+ });
581+ },
582+ },
583+ ]);
584+
585+ expect(registerLateFinalizer).toThrow("no longer active");
586+ await runtime.dispose();
587+ expect(disposed).toBe(false);
588+ });
589+
590+ it("snapshots the requested definitions before waiting for an earlier transition", async () => {
591+ let releaseActivation!: () => void;
592+ const activationGate = new Promise<void>((resolve) => {
593+ releaseActivation = resolve;
594+ });
595+ const runtime = createRuntime();
596+ const first = runtime.reconcile([
597+ {
598+ id: "acme.blocking",
599+ version: "1.0.0",
600+ async activate() {
601+ await activationGate;
602+ },
603+ },
604+ ]);
605+ const requested: Array<PluginDefinition> = [provider()];
606+ const second = runtime.reconcile(requested);
607+ requested.splice(0);
608+
609+ releaseActivation();
610+ await first;
611+ const snapshot = await second;
612+
613+ expect(snapshot.active).toEqual(["acme.database"]);
538614 await runtime.dispose();
539615 });
540616