Skip to content

[Repo Assist] Fix CircularBuffer.Enqueue(array) - head pointer advances by count, not 1 #233

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

Merged
gdziadkiewicz merged 5 commits into from
Mar 29, 2026
+46 −5
Merged

[Repo Assist] Fix CircularBuffer.Enqueue(array) - head pointer advances by count, not 1 #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a47df54
Fix CircularBuffer.Enqueue(array) advancing head by count instead of 1
github-actions[bot] Mar 8, 2026
624dffc
ci: trigger checks
github-actions[bot] Mar 8, 2026
301ff31
Merge branch 'master' into repo-assist/fix-issue-125-circular-buffer-…
gdziadkiewicz Mar 29, 2026
028c9f9
Add offset/count input validation to CircularBuffer.Enqueue and tests
Copilot Mar 29, 2026
cbcc08c
Merge branch 'master' into repo-assist/fix-issue-125-circular-buffer-…
gdziadkiewicz Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/FSharpx.Collections/CircularBuffer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,31 @@ type CircularBuffer<'T>(bufferSize: int) =
dequeued

member this.Enqueue(value: _[], offset, count) =
if isNull value then
invalidArg "value" "value must not be null."

if offset < 0 then
invalidArg "offset" "offset must not be negative."

if count < 0 then
invalidArg "count" "count must not be negative."

if offset + count > value.Length then
invalidArg "count" "offset and count exceed the length of the source array."

if count > bufferSize then
invalidOp "Requested count is too large."

let mutable offset = offset

head <- (head + 1) % bufferSize
let startPos = (head + 1) % bufferSize

for x, y in nextBuffer head count do
for x, y in nextBuffer startPos count do
Array.blit value offset buffer x y
offset <- offset + y
Comment on lines +68 to 72
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

Enqueue(value: _[], offset, count) doesn't validate offset/count against value.Length (and non-negative). If Array.blit throws (e.g., offset + count > value.Length), the buffer may already be partially overwritten (especially when length = bufferSize), leaving the collection in a corrupted state. Consider validating inputs upfront so the operation either completes fully or fails without mutating the underlying buffer.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@gdziadkiewicz gdziadkiewicz Mar 29, 2026

Choose a reason for hiding this comment

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

@copilot Add validation of count and offset and add tests for it

Copy link
Copy Markdown
Contributor

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

Added in commit 028c9f9. Enqueue(value, offset, count) now validates upfront (before any mutation):

  • value is not null
  • offset is non-negative
  • count is non-negative
  • offset + count does not exceed value.Length

Three new tests cover each of the invalid-argument cases.


head <- (head + count) % bufferSize

if length = bufferSize then
tail <- (tail + count) % bufferSize
else
Expand Down
33 changes: 30 additions & 3 deletions tests/FSharpx.Collections.Tests/CircularBufferTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,34 @@ module CircularBufferTests =
Expect.throwsT<System.InvalidOperationException> "" f
}

ptest "Printing after multiple enqueue circles" {
test "fail on negative offset" {
let f =
fun _ ->
let circularBuffer = CircularBuffer<int> 5
circularBuffer.Enqueue([| 1; 2; 3 |], -1, 2)

Expect.throwsT<System.ArgumentException> "" f
}

test "fail on negative count" {
let f =
fun _ ->
let circularBuffer = CircularBuffer<int> 5
circularBuffer.Enqueue([| 1; 2; 3 |], 0, -1)

Expect.throwsT<System.ArgumentException> "" f
}

test "fail when offset + count exceeds array length" {
let f =
fun _ ->
let circularBuffer = CircularBuffer<int> 5
circularBuffer.Enqueue([| 1; 2; 3 |], 2, 3)

Expect.throwsT<System.ArgumentException> "" f
}

test "Printing after multiple enqueue circles" {
let circularBuffer = CircularBuffer<int> 5

circularBuffer.Enqueue [| 1; 2; 3; 4; 5 |]
Expand All @@ -96,7 +123,7 @@ module CircularBufferTests =



ptest "Printing from a queue 1..8 and dequeue 5, then enqueue 1..3 and dequeue 3, from array" {
test "Printing from a queue 1..8 and dequeue 5, then enqueue 1..3 and dequeue 3, from array" {
let circularBuffer = CircularBuffer<int> 5

circularBuffer.Enqueue([| 1; 2; 3; 4; 5 |])
Expand All @@ -106,7 +133,7 @@ module CircularBufferTests =
Expect.equal "buffer" [| 1; 2; 3 |] <| circularBuffer.Dequeue 3
}

ptest "Consider a large array with various, incoming array segments" {
test "Consider a large array with various, incoming array segments" {
let circularBuffer = CircularBuffer<int> 5

let source =
Expand Down
Loading