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

MQE: mangle native histograms in returned slices to catch use-after-return bugs + fix issues found #10010

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Nov 25, 2024

What this PR does

This PR extends MQE's tests to aid in catching native histogram use-after-return bugs, and fixes some issues they've uncovered.

Now, during tests in the streamingpromql package, any histograms in slices returned to the HPoint or FloatHistogram pools will immediately be mutated. This ensures that any possible use-after-return bugs are triggered deterministically.

Which issue(s) this PR fixes or relates to

(none)

Checklist

  • Tests updated.
  • [n/a] Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [n/a] about-versioning.md updated with experimental features.

@charleskorn charleskorn marked this pull request as ready for review November 25, 2024 05:28
@charleskorn charleskorn requested a review from a team as a code owner November 25, 2024 05:28

h.ZeroCount = 12345678
h.Count = 12345678
h.Sum = 12345678
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) it's very unlikely, but if we write a test with these values elsewhere the mangle won't work. Maybe for just one of these (eg Sum) we multiply it or similar.

Suggested change
h.Sum = 12345678
h.Sum = h.Sum * 9.87654321

}

func NewLimitingBucketedPool[S ~[]E, E any](inner *pool.BucketedPool[S, E], elementSize uint64, clearOnGet bool) *LimitingBucketedPool[S, E] {
func NewLimitingBucketedPool[S ~[]E, E any](inner *pool.BucketedPool[S, E], elementSize uint64, clearOnGet bool, mangle func(E) E) *LimitingBucketedPool[S, E] {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add a quick test that mangle func(E) E is called when the setting is true/false etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants