Skip to content

fix: honor allocation limits at boundary cases - #2

Merged
kolkov merged 1 commit into
gogpu:mainfrom
besmpl:agent/fix-allocation-boundaries
Aug 11, 2026
Merged

fix: honor allocation limits at boundary cases#2
kolkov merged 1 commit into
gogpu:mainfrom
besmpl:agent/fix-allocation-boundaries

Conversation

@besmpl

@besmplbesmpl commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reserve enough internal region nodes to honor the documented maxAllocs count even when allocations split free regions
  • track active allocations separately from structural metadata and keep fragmented allocation/coalescing O(1)
  • reject aligned requests whose padded size would overflow uint32
  • add one-slot, zero-capacity, fragmented-capacity, reuse, accounting, overflow, and metadata-exhaustion regressions

Why

The initial free region used one of the advertised allocation nodes. As a result, New(1024, 1).Allocate(1) attempted to create a remainder through the freelist sentinel and panicked instead of returning the single supported allocation. Fragmented layouts could similarly exhaust structural nodes before reaching maxAllocs.

Free regions are never adjacent after coalescing, so with K active allocations there are at most K+1 free regions. A preallocated 2*maxAllocs node pool (or one initial node for a zero-capacity allocator) therefore covers every reachable layout while the new active count enforces the public limit.

Separately, AllocateAligned previously allowed size + alignment - 1 to wrap, potentially returning a tiny successful allocation for a near-MaxUint32 request.

Verification

  • go test ./...
  • go test -race ./...
  • go build ./...
  • go vet ./...
  • golangci-lint run --timeout=5m
  • go test -covermode=atomic -coverprofile=coverage.out ./... — 99.5% total statement coverage and 100% of changed production statements (42/42; no uncovered changed blocks)
  • randomized aligned and unaligned fragmentation stress
  • allocator fuzz smoke tests
  • hot-path benchmarks remain 0 B/op, 0 allocs/op
  • gofmt and git diff --check

CI note: GitHub marks this fork pull request's workflow run as action_required; upstream Actions and the Codecov upload will start after a repository maintainer approves the run.

@besmpl
besmpl marked this pull request as ready for review August 10, 2026 17:04
@besmpl
besmpl requested a review from kolkov as a code ownerAugust 10, 2026 17:04
@besmpl
besmplforce-pushed the agent/fix-allocation-boundaries branch from b94b8c3 to 1e28061CompareAugust 10, 2026 21:15

@kolkovkolkov 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.

@besmpl — thorough fix. Validated locally:

Bug confirmed:New(1024, 1).Allocate(1) panics on main — insertNodeIntoBin crashes because the initial free region consumed the only allocation node.

Reference checked: The original C++ OffsetAllocator has the same bug — allocates exactly maxAllocs nodes (line 184), and the initial free region at line 195 consumes one, leaving zero usable slots when maxAllocs=1. Your fix addresses a bug in the upstream reference implementation itself.

Validation:

CheckResult
go build ./...
go test ./... -count=1✅ all pass
go vet ./...
golangci-lint run --timeout=5m✅ 0 issues
Math proof (2K nodes for K allocs + K+1 free regions)✅ sound
Overflow guard size > NoSpace-(alignment-1)✅ prevents uint32 wrap
No API break

Approving. Will merge after CI passes.

@codecov

codecovBot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kolkov
kolkov merged commit d3cdda4 into gogpu:mainAug 11, 2026
10 checks passed
Sign up for freeto 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

@besmpl@kolkov