[WIP] index header bucket reader async readahead scratch r409 - #16498
[WIP] index header bucket reader async readahead scratch r409#16498francoposa wants to merge 9 commits into
Conversation
… sliding unconsumed peekBuf bytes;implement bucket read cancellation for buffer promise release; convert resetReader func from bucket reader to something normal
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca47b41. Configure here.
| readN, err := io.ReadFull(headPromise, bbar.peekBuf[peekBytesWritten:peekBytesWritten+toRead]) | ||
| peekBytesWritten += readN | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Failed Peek corrupts buffered state
High Severity
Peek grows peekBuf to the requested length before the fill completes. On a promise error it returns without shrinking peekBuf back to the bytes actually copied. The next Peek, Read, or Skip treats those unfilled bytes as valid data, so a retry can succeed with zeros or stale tail bytes and shift the stream.
Reviewed by Cursor Bugbot for commit ca47b41. Configure here.
| bbar.readOffset += bytesSkipped | ||
|
|
||
| // Slide any unconsumed bytes from peekBuf to the beginning of the slice and truncate. | ||
| n := copy(bbar.peekBuf, bbar.peekBuf[bytesSkipped:]) |
There was a problem hiding this comment.
Negative Skip and Peek panic
High Severity
Skip and Peek only reject counts larger than Len(), so a negative count passes the check. Skip then slices peekBuf at a negative index and panics; Peek clamps n to a negative value via min and panics on peekBuf[:n]. Decbuf.SkipUvarintBytes can pass a negative int when a corrupt varint overflows, so a bad index header can crash the process.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ca47b41. Configure here.


What this PR does
Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]. If changelog entry is not needed, please add thechangelog-not-neededlabel to the PR.about-versioning.mdupdated with experimental features.