feat(kernel): Add removable bootstrap keep‑alive pin #502
Conversation
There was a problem hiding this comment.
This looks very good but I do have one question raised below. I'm approving so you can go ahead and land the PR if your take on the question is to go with things as they are here, but please do at least give the matter some thought before pressing the big green button.
| const pinList = getPinnedObjects(); | ||
| const pinned = new Set(pinList); | ||
| if (!pinned.has(kref)) { | ||
| incrementRefCount(kref, 'pin'); | ||
| pinned.add(kref); | ||
| ctx.kv.set('pinnedObjects', [...pinned].sort().join(',')); | ||
| } |
There was a problem hiding this comment.
This is how SwingSet does it, but I think it's worth asking ourselves if we really want pinning to be boolean or not. In the implementation here, if two different, unrelated parts of the code each call pinObject (for their own reasons) it will still only get pinned once, and then if each later calls unpinObject, the object will be unpinned by the first call and subject to GC even though the second chunk of code might not yet be ready for that GC to happen.
This is probably an academic question since the primary use cases for pinning are tests and bootstrap vats, where in both cases pinning tends to be monotonic anyway, but I'd like us to at least think about the question.
There was a problem hiding this comment.
Yes, your comment is valid, even more since SwingSet doesn't have an unpinObject method. Change done 582537a
Closes #494
This PR implements a
pinand anunpinmethod for vat root objects, that external code (tests, control panel) can call to keep a vat or arbitrary kernel object alive to ensure that references are counted.A small reorganize of the kernel store methods was required and I added some tests that were missing.