From 1355e603c048573806003b34d085bdc04a828baa Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 15 Sep 2026 17:31:30 +0200 Subject: [PATCH] fix(ocap-kernel): sort GC krefs before hardening them `processGCActionSet` sorted the kref list `filterActionsForProcessing` had already hardened, so any endpoint with two pending actions of one type threw a TypeError out of the run loop. Co-Authored-By: Claude Fable 5.1 --- packages/ocap-kernel/CHANGELOG.md | 1 + .../garbage-collection.test.ts | 20 +++++++++++++++++++ .../garbage-collection/garbage-collection.ts | 9 ++++----- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 1393fc21a5..47264fc7fa 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -61,6 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Two pending garbage-collection actions of one type for one endpoint no longer kill the run loop: `processGCActionSet` sorted the kref list after hardening it, and sorting a frozen array throws ([#1082](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1082)) - A message delivered to a kernel-owned kref with no registered service now rejects the caller with `ENDPOINT_UNREACHABLE` instead of throwing, which escaped the crank and killed the run loop — turning one unreachable reference into a dead kernel ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Reachable without any kernel bug: an anonymous kernel object hosts something that cannot outlive the process, such as an accepted socket connection, so a vat holding one across a restart or a message to one still queued from the previous incarnation lands here. That surviving reference is exactly what stops the init sweep deleting the object, so its `kernel` owner survives with it - Matches what `KernelRouter` already does for a delivery whose endpoint has vanished. A message sent with no result promise has nobody to report to, so it is logged instead diff --git a/packages/ocap-kernel/src/garbage-collection/garbage-collection.test.ts b/packages/ocap-kernel/src/garbage-collection/garbage-collection.test.ts index bca1752652..90675accd3 100644 --- a/packages/ocap-kernel/src/garbage-collection/garbage-collection.test.ts +++ b/packages/ocap-kernel/src/garbage-collection/garbage-collection.test.ts @@ -84,6 +84,26 @@ describe('garbage-collection', () => { expect(kernelStore.getGCActions().size).toBe(0); }); + it('groups two actions of one type for one vat into a single item', () => { + const ko1 = kernelStore.initKernelObject('v1'); + const ko2 = kernelStore.initKernelObject('v1'); + kernelStore.addCListEntry('v1', ko1, 'o+1'); + kernelStore.addCListEntry('v1', ko2, 'o+2'); + kernelStore.setObjectRefCount(ko1, { reachable: 0, recognizable: 1 }); + kernelStore.setObjectRefCount(ko2, { reachable: 0, recognizable: 1 }); + kernelStore.addGCActions([ + `v1 dropExport ${ko2}`, + `v1 dropExport ${ko1}`, + ]); + + expect(processGCActionSet(kernelStore)).toStrictEqual({ + type: 'dropExports', + endpointId: 'v1', + krefs: [ko1, ko2], + }); + expect(kernelStore.getGCActions().size).toBe(0); + }); + it('processes actions in priority order', () => { // Setup: Create objects and add multiple GC actions const ko1 = kernelStore.initKernelObject('v1'); diff --git a/packages/ocap-kernel/src/garbage-collection/garbage-collection.ts b/packages/ocap-kernel/src/garbage-collection/garbage-collection.ts index 5bd107793c..5f7f65711b 100644 --- a/packages/ocap-kernel/src/garbage-collection/garbage-collection.ts +++ b/packages/ocap-kernel/src/garbage-collection/garbage-collection.ts @@ -93,7 +93,7 @@ function filterActionsForProcessing( endpointId: EndpointId, actions: Set, allActionsSet: Set, -): { krefs: KRef[]; actionSetUpdated: boolean } { +): { krefs: readonly KRef[]; actionSetUpdated: boolean } { const krefs: KRef[] = []; let actionSetUpdated = false; @@ -106,6 +106,8 @@ function filterActionsForProcessing( actionSetUpdated = true; } + // `harden` freezes `krefs`, so it has to be sorted first. + krefs.sort(); return harden({ krefs, actionSetUpdated }); } @@ -174,16 +176,13 @@ export function processGCActionSet( actionSetUpdated = actionSetUpdated || updated; if (krefs.length > 0) { - // We found actions to process - krefs.sort(); - // Update the durable set before returning storage.setGCActions(allActionsSet); const queueType = queueTypeFromActionType.get(type); assert(queueType !== undefined, `Unknown action type: ${type}`); - return harden({ type: queueType, endpointId, krefs }); + return harden({ type: queueType, endpointId, krefs: [...krefs] }); } } }