From c44d872d66fe34bc2cb9cfd1bfb4250d8adc9fec Mon Sep 17 00:00:00 2001 From: Mark Fields Date: Fri, 6 Jun 2025 17:40:26 +0000 Subject: [PATCH 1/2] Close the container if exitStagingMode throws --- .../container-runtime/src/containerRuntime.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index ca535c7f3e1a..1808486a6c0a 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -3474,22 +3474,28 @@ export class ContainerRuntime throw new UsageError("cannot enter staging mode while detached"); } - // Make sure all BatchManagers are empty before entering staging mode, + // Make sure Outbox is empty before entering staging mode, // since we mark whole batches as "staged" or not to indicate whether to submit them. - this.outbox.flush(); + this.flush(); const exitStagingMode = (discardOrCommit: () => void) => (): void => { - // Final flush of any last staged changes - this.outbox.flush(); + try { + // Final flush of any last staged changes + this.flush(); - this.stageControls = undefined; + this.stageControls = undefined; - // During Staging Mode, we avoid submitting any ID Allocation ops (apart from resubmitting pre-staging ops). - // Now that we've exited, we need to submit an ID Allocation op for any IDs that were generated while in Staging Mode. - this.submitIdAllocationOpIfNeeded({ staged: false }); - discardOrCommit(); + // During Staging Mode, we avoid submitting any ID Allocation ops (apart from resubmitting pre-staging ops). + // Now that we've exited, we need to submit an ID Allocation op for any IDs that were generated while in Staging Mode. + this.submitIdAllocationOpIfNeeded({ staged: false }); + discardOrCommit(); - this.channelCollection.notifyStagingMode(false); + this.channelCollection.notifyStagingMode(false); + } catch (error) { + const normalizedError = normalizeError(error); + this.closeFn(normalizedError); + throw normalizedError; + } }; // eslint-disable-next-line import/no-deprecated From c63f6f62c2eb638b8642a6bb52bf338b7fb7634c Mon Sep 17 00:00:00 2001 From: Mark Fields Date: Tue, 10 Jun 2025 22:05:05 +0000 Subject: [PATCH 2/2] Revert back to calling outbox.flush directly in exitStagingMode this.flush is incompatible with batchRunner. I would rather structure orderSequentially to be able to handle staging mode using this.flush, but it's not the priority right now. --- packages/runtime/container-runtime/src/containerRuntime.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 1808486a6c0a..d9ead1d39cfe 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -3481,7 +3481,8 @@ export class ContainerRuntime const exitStagingMode = (discardOrCommit: () => void) => (): void => { try { // Final flush of any last staged changes - this.flush(); + // NOTE: We can't use this.flush() here, because orderSequentially uses StagingMode and in the rollback case we'll hit assert 0x24c + this.outbox.flush(); this.stageControls = undefined;