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

fix(liveslots): collection.clear(pattern) vs getSize #10008

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

warner
Copy link
Member

@warner warner commented Aug 31, 2024

The .clear() method, on (strong) collections, will delete all entries. But it can also be called with keyPatt and/or valuePatt argument, and only the matching entries will be deleted.

Previously, the |entryCount metadata field was unconditionally set to zero for any call to .clear(), even those with patterns which caused some entries to be retained. This caused a subsequent collection.getSize() to return the wrong value, yielding zero even if the collection still had entries.

This commit fixes the logic to decrement the |entryCount field by the number of entries actually deleted. As an optimization, it is still set directly to zero if clear() is not limited by patterns.

fixes #10007

@warner warner requested a review from mhofman August 31, 2024 00:42
Copy link

cloudflare-workers-and-pages bot commented Aug 31, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: f8806f3
Status: ✅  Deploy successful!
Preview URL: https://63f9e408.agoric-sdk.pages.dev
Branch Preview URL: https://warner-10007-collection-clea.agoric-sdk.pages.dev

View logs

Base automatically changed from warner/8756-collection-deletion-gc-bug to master August 31, 2024 01:40
Copy link

Base branch is changed to master. Please re-run the integration tests by adding 'force:integration' label.

@warner warner added the automerge:rebase Automatically rebase updates, then merge label Aug 31, 2024
The `.clear()` method, on (strong) collections, will delete all entries.
But it can also be called with `keyPatt` and/or `valuePatt` argument,
and only the matching entries will be deleted.

Previously, the `|entryCount` metadata field was unconditionally set
to zero for any call to `.clear()`, even those with patterns which
caused some entries to be retained. This caused a subsequent
`collection.getSize()` to return the wrong value, yielding zero even
if the collection still had entries.

This commit fixes the logic to decrement the `|entryCount` field by
the number of entries actually deleted. As an optimization, it is
still set directly to zero if `clear()` is not limited by patterns.

fixes #10007
@warner warner force-pushed the warner/10007-collection-clear-pattern branch from 1c10d6d to f8806f3 Compare August 31, 2024 01:53
@mergify mergify bot merged commit 17c73b7 into master Aug 31, 2024
80 checks passed
@mergify mergify bot deleted the warner/10007-collection-clear-pattern branch August 31, 2024 02:26
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collection.clear(pattern) corrupts entryCount by setting it to zero
2 participants