Skip to content

Fix Meta() panic when a source is truncated mid-call - #613

Merged
chombium merged 1 commit into
cloudfoundry:mainfrom
dlinsley:fix/meta-count
Oct 22, 2026
Merged

chombium merged 1 commit into
cloudfoundry:mainfrom
dlinsley:fix/meta-count

Conversation

@dlinsley

Copy link
Copy Markdown
Contributor

Description

Store.Meta() made two passes over storageIndex. The second pass re-loaded each source ID and asserted the result to *storage without checking whether the entry still existed. If the truncation loop deleted a source between the two passes, Load returned nil and the type assertion panicked.

The second pass existed only because meta.Count was never populated; it overwrote Count with tree.Size(). Maintain meta.Count on the storage whenever the tree changes (insertOrSwap and removeOldestEnvelope) and read it directly in Meta(). Meta() now makes a single pass and no longer re-loads from storageIndex.

Also resolved the goal of the TODO comment

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

If you have any questions, or want to get attention for a PR or issue please reach out on the #logging-and-metrics channel in the cloudfoundry slack

Store.Meta() made two passes over storageIndex. The second pass
re-loaded each source ID and asserted the result to *storage without
checking whether the entry still existed. If the truncation loop
deleted a source between the two passes, Load returned nil and the
type assertion panicked.

The second pass existed only because meta.Count was never populated;
it overwrote Count with tree.Size(). Maintain meta.Count on the
storage whenever the tree changes (insertOrSwap and
removeOldestEnvelope) and read it directly in Meta(). Meta() now
makes a single pass and no longer re-loads from storageIndex.

Also resolved the goal of the TODO comment

@chombium chombium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!
Nice work @dlinsley and one nasty edge case bug is gone.

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Oct 22, 2026
@chombium
chombium merged commit d71f0c8 into cloudfoundry:main Oct 22, 2026
5 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants