Skip to content

fix: runtime panic "index out of range" caused by nextCleanupBucket #32

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 3, 2024
Merged

fix: runtime panic "index out of range" caused by nextCleanupBucket #32

merged 1 commit into from
Apr 3, 2024

Conversation

Laisky
Copy link
Contributor

@Laisky Laisky commented Apr 3, 2024

The type of nextCleanupBucket is int8, if the size is greater than int8, it will result in overflow causing negative numbers, ultimately leading to a program panic.

panic: runtime error: index out of range [-28]

goroutine 104 [running]:
github.com/scalalang2/golang-fifo/sieve.(*Sieve[...]).addToBucket(0xe3ae20, 0xc000496b90)
	/home/laisky.cai/go/pkg/mod/github.com/scalalang2/[email protected]/sieve/sieve.go:250 +0x3a7
github.com/scalalang2/golang-fifo/sieve.(*Sieve[...]).Set(0xe3ae20, {0xc000346468, 0x8}, {0xc000346478, 0x8})
	/home/laisky.cai/go/pkg/mod/github.com/scalalang2/[email protected]/sieve/sieve.go:117 +0xb91

@Laisky
fix: runtime error index out of range caused by nextCleanupBucket
49319aa 
- Adjusted cache size and TTL in `sieve_test.go` for more comprehensive testing
- Implemented a constant `numberOfBuckets` and utilized it in `sieve.go` for cache storage
- Updated `addToBucket` function to incorporate the new `numberOfBuckets` constant and `int8` `bucketID` type
- Modified `bucketId` calculation in `addToBucket` function using `s.nextCleanupBucket` of type `float64`
- Enhanced workload for testing in `sieve_test.go` from 256 to 10240 for rigorous testing
scalalang2
scalalang2 reviewed Apr 3, 2024
View reviewed changes
Copy link
Owner

@scalalang2 scalalang2 left a comment

Choose a reason for hiding this comment

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

thanks for a new contribution :) LGTM.

@scalalang2
Copy link
Owner

scalalang2 commented Apr 3, 2024 •
edited
Loading

This changes makes sense to me.
Thanks for this improvement 👍

@scalalang2 scalalang2 merged commit ef5cb24 into scalalang2:main Apr 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scalalang2 scalalang2 scalalang2 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Laisky @scalalang2