kernel: fix lingering ref‑counts that blocked GC #492
Conversation
sirtimid
force-pushed
the
sirtimid/fix-lingering-ref-counts-gc
branch
from
123a57b to
4ab5b1c
Compare
FUDCo
left a comment
FUDCo
left a comment
There was a problem hiding this comment.
Mostly excellent. One possible anti-leak that needs to be either fixed or explained.
| await this.#runVat(vatId, vatConfig); | ||
| this.#kernelStore.initEndpoint(vatId); | ||
| const rootRef = this.#kernelStore.exportFromVat(vatId, ROOT_OBJECT_VREF); | ||
| this.#kernelStore.incrementRefCount(rootRef, 'root'); |
There was a problem hiding this comment.
I don't think we need to pin every root ref (in fact, we absolutely shouldn't) but something like this has to happen somewhere (perhaps launchSubcluster?) to ensure that references (e.g., to bootstrap vats) that are only held externally (e.g., by the control panel or test scripts) but never explicitly exported via a message parameter are counted, else one's whole subcluster could simply disappear after a GC. (Speaking cosmologically, the universe as a whole is unreferenced.). This can sneak up on you (ask me how I know :-)
There was a problem hiding this comment.
I pinky swear that a whole subcluster can't disappear except if one deletes the bootstrap vat after they have deleted all the other vats and have garbage collected them. Shouldn't we support this process? One can't just delete all vats (with the bootstrap) and garbage collect them, the references brake. In other words: the requirement that you retire the secondary vats before you retire the bootstrap is what guarantees safety imo.
Anyway I suggest we do this on another "add removable bootstrap keep‑alive pin" task and perhaps follow agorics convention of pin/unpin. wdyt?
There was a problem hiding this comment.
Yup, that that sounds like the right plan to me.
There was a problem hiding this comment.
Oh, subtle point:
It's not that the vat itself gets GC'd, it's that the vat's root object gets GC'd, so unless the vat has in the meantime wired itself up with other vats you lose the ability to talk to it.
There was a problem hiding this comment.
BTW, as a general rule, GC'ing a vat's root object while the rest of the vat continues to run is not a problem. At Agoric, it was not uncommon to create a vat with an ephemeral root object whose only job was to initialize things and then go away.
There was a problem hiding this comment.
Ha! I didn't expect that, but, yes of course, why not!
This patch closes the last reference‑count leaks that left fulfilled promises and exported root objects in the store after vats were terminated or the kernel was restarted, ensuring the run‑queue, promises, and object graphs now cleanly reach zero and are fully reclaimed by GC.