Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

collectionManager: refcount remotables in keyShape/valueShape #7334

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

warner
Copy link
Member

@warner warner commented Apr 5, 2023

collectionManager: refcount remotables in keyShape/valueShape

The collection schema (keyShape/valueShape) can refer to specific
Remotable instances (e.g. Presence, or a durable object), rather than
defining a constraint to be a whole category of objects. The
collection schema metadata is thus a form of write-once state storage,
like a tiny virtual object.

Previously, defineKind failed to increment the refcount on the
objects' vrefs, so when the collection falls out of memory, the object
might be deleted, and then we would be unable to revive the constraint
when loading the schema back in later.

This changes the code to increment the refcounts of the vrefs in
keyShape and valueShape when the collection is created, and decrement
them when the collection is deleted. It handles ephemeral Remotables,
as well as virtual/durable object Representatives. For durable
collections, it also enforces the same isDurable check on the schema
as it would on keys and values of the collection itself, so you cannot
use a Far() or a merely-virtual object in the valueShape of a
durable collection.

closes #7321

@warner warner added SwingSet package: SwingSet liveslots requires vat-upgrade to deploy changes labels Apr 5, 2023
@warner warner requested review from gibson042 and FUDCo April 5, 2023 08:32
@warner warner self-assigned this Apr 5, 2023
@warner warner removed the request for review from FUDCo April 5, 2023 08:32
@warner
Copy link
Member Author

warner commented Apr 5, 2023

Two of the new tests are disabled because I haven't been able to figure out how to avoid the "unhandled rejection" error from flunking the test. @gibson042 I could probably use your help figuring out that one.

@warner warner changed the base branch from master to 6360-schema-cache April 5, 2023 08:34
@warner
Copy link
Member Author

warner commented Apr 5, 2023

hrm, something is failing in a packages/vats upgrade test, feels like the refcounts are causing something to get deleted by mistake, will investigate tomorrow

@warner warner force-pushed the 7321-refcount-keyshape-vrefs branch 3 times, most recently from 93cb8ae to b147977 Compare April 7, 2023 06:43
@warner warner requested a review from FUDCo April 7, 2023 06:53
@warner warner force-pushed the 7321-refcount-keyshape-vrefs branch from b147977 to c260cc8 Compare April 9, 2023 03:34
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Possibly also a new contender for largest ratio of test size to code change size .

packages/swingset-liveslots/src/collectionManager.js Outdated Show resolved Hide resolved
@warner warner force-pushed the 6360-schema-cache branch from 082f244 to 83155ef Compare April 10, 2023 20:49
@warner warner force-pushed the 7321-refcount-keyshape-vrefs branch 4 times, most recently from a9b2fad to db996d0 Compare April 11, 2023 02:06
Base automatically changed from 6360-schema-cache to master April 11, 2023 02:48
@warner warner added the automerge:rebase Automatically rebase updates, then merge label Apr 11, 2023
warner added 2 commits April 11, 2023 02:50
The collection schema (keyShape/valueShape) can refer to specific
Remotable instances (e.g. Presence, or a durable object), rather than
defining a constraint to be a whole category of objects. The
collection schema metadata is thus a form of write-once state storage,
like a tiny virtual object.

Previously, defineKind failed to increment the refcount on the
objects' vrefs, so when the collection falls out of memory, the object
might be deleted, and then we would be unable to revive the constraint
when loading the schema back in later.

This changes the code to increment the refcounts of the vrefs in
keyShape and valueShape when the collection is created, and decrement
them when the collection is deleted. It handles ephemeral Remotables,
as well as virtual/durable object Representatives. For durable
collections, it also enforces the same `isDurable` check on the schema
as it would on keys and values of the collection itself, so you cannot
use a `Far()` or a merely-virtual object in the valueShape of a
durable collection.

closes #7321
Check that the refcounts are maintained correctly
@warner warner force-pushed the 7321-refcount-keyshape-vrefs branch from db996d0 to 5af1b31 Compare April 11, 2023 02:51
@mergify mergify bot merged commit 70ce64b into master Apr 11, 2023
@mergify mergify bot deleted the 7321-refcount-keyshape-vrefs branch April 11, 2023 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge liveslots requires vat-upgrade to deploy changes SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collections permit Presences/vrefs in keyShape/valueShape, but doesn't refcount them
2 participants