Skip to content

Fix -race checkptr fatal: replace global unsafe.Pointer expunged sentinel with per-entry typed field#182

Merged
kpango merged 1 commit intomainfrom
copilot/fix-checkptr-race-issue
Mar 23, 2026
Merged

Fix -race checkptr fatal: replace global unsafe.Pointer expunged sentinel with per-entry typed field#182
kpango merged 1 commit intomainfrom
copilot/fix-checkptr-race-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

  • Remove global var expunged = unsafe.Pointer(new(any))
  • Add expunged atomic.Pointer[V] field to entry[V] struct
  • Initialize expunged with new(V) in newEntry and newEntryPointer
  • Add isExpunged(p *V) bool helper method on entry[V]
  • Replace all (*V)(expunged) casts with e.expunged.Load() / e.isExpunged(p)
  • All tests pass with -race flag
  • PR diff cleaned up — only map.go changed (removed unrelated Makefile/go.mod/go.sum/counter changes)
Original prompt

Problem

Resolves #181

Since v2.1.5, running tests with -race flag triggers a fatal checkptr error:

fatal error: checkptr: converted pointer straddles multiple allocations
at github.com/kpango/gache/v2/map.go:139

The root cause is the use of a global untyped sentinel value:

var expunged = unsafe.Pointer(new(any))

This is then cast to (*V)(expunged) in many places throughout map.go. When V is a non-pointer value type (e.g. a struct), the cast (*V)(expunged) creates an unsafe pointer conversion across different memory allocations, violating Go's pointer safety rules that are checked when -race is enabled.

Required Fix

Replace the global unsafe.Pointer sentinel with a per-entry typed expunged pointer stored as an atomic.Pointer[V] field in the entry[V] struct. This makes all expunged comparisons type-safe.

Changes needed in map.go:

  1. Remove the global variable:

    var expunged = unsafe.Pointer(new(any))
  2. Add an expunged field to the entry[V] struct:

    type entry[V any] struct {
        expunged atomic.Pointer[V]
        p        atomic.Pointer[V]
    }
  3. Initialize expunged in newEntry and newEntryPointer:

    func newEntry[V any](v V) (e *entry[V]) {
        e = &entry[V]{}
        e.expunged.Store(new(V))
        e.p.Store(&v)
        return e
    }
    
    func newEntryPointer[V any](v *V) (e *entry[V]) {
        e = &entry[V]{}
        e.expunged.Store(new(V))
        e.p.Store(v)
        return e
    }
  4. Add a helper method isExpunged on entry[V]:

    func (e *entry[V]) isExpunged(p *V) bool {
        return p == e.expunged.Load()
    }
  5. Replace all occurrences of (*V)(expunged) with e.expunged.Load() and all p == (*V)(expunged) comparisons with e.isExpunged(p). This affects the following methods:

    • entry[V].load() (line 131)
    • entry[V].loadPointer() (line 139)
    • entry[V].unexpungeLocked() (line 174-175)
    • entry[V].tryLoadOrStorePointer() (lines 295, 307)
    • entry[V].deletePointer() (line 339)
    • entry[V].trySwap() (line 355)
    • entry[V].tryCompareAndSwap() (lines 412, 422)
    • entry[V].tryCompareAndSwapPointer() (line 430)
    • entry[V].tryExpungeLocked() (lines 528, 533)
    • entry[V].Size() (line 564)
    • Map[K,V].CompareAndDelete() (line 454)
  6. For unexpungeLocked, change:

    func (e *entry[V]) unexpungeLocked() (wasExpunged bool) {
        return e.p.CompareAndSwap(e.expunged.Load(), nil)
    }
  7. For tryExpungeLocked, change:

    func (e *entry[V]) tryExpungeLocked() (isExpunged bool) {
        p := e.p.Load()
        for p == nil {
            if e.p.CompareAndSwap(nil, e.expunged.Load()) {
                return true
            }
            p = e.p.Load()
        }
        return e.isExpunged(p)
    }
  8. Remove the import "unsafe" if it's no longer needed (it is still used in Size() methods via unsafe.Sizeof, so keep it).

Important: The import "unsafe" must be kept because it's still used in Size() and mapSize().

Make sure all existing tests pass, especially with -race flag. The key insight is that every entry[V] now carries its own properly-typed expunged sentinel, so no cross-allocation pointer casts occur.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Can you resolve issue @kpango/gache/issues/181 ?

Assistant:

Let me start by examining the current codebase to understand the issue and the relevant code.

Assistant: Now I have a clear understanding of the issue and the codebase. Let me create a PR to fix this.

Assistant: I've initiated the creation of a pull request to resolve issue #181 in kpango/gache. The coding agent is now working on implementing the fix. Here's a summary of what the PR will do:

Root Cause

The global untyped sentinel var expunged = unsafe.Pointer(new(any)) is cast to (*V)(expunged) throughout map.go. When V is a non-pointer value type (like a struct), this cast creates an unsafe pointer conversion that straddles multiple allocations — which Go's checkptr instrumentation (enabled with -race) correctly flags as a fatal error.

Fix Approach

The PR will replace the global unsafe.Pointer sentinel with a per-entry typed expunged pointer stored as an atomic.Pointer[V] field in the entry[V] struct:

  1. Remove var expunged = unsafe.Pointer(new(any))
  2. Add expunged atomic.Pointer[V] field to entry[V]
  3. Initialize it with new(V) in newEntry/newEntryPointer (type-safe allocation)
  4. Add an isExpunged(p *V) bool helper method
  5. Replace all (*V)(expunged) casts with e.expunged.Load() across ~12 locations in the file

This ensures all expunged comparisons are type-safe — no unsafe.Pointer casts crossing allocation boundaries — while preserving the exact same concurrent map semantics.

The agent wil...

This pull request was created from Copilot chat.


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Fix checkptr error triggered by race flag in tests Fix -race checkptr fatal: replace global unsafe.Pointer expunged sentinel with per-entry typed field Mar 18, 2026
Copilot AI requested a review from kpango March 18, 2026 22:25
@kpango kpango force-pushed the copilot/fix-checkptr-race-issue branch 5 times, most recently from af31628 to 9cd6261 Compare March 18, 2026 22:37
@kpango kpango marked this pull request as ready for review March 18, 2026 22:37
@kpango kpango force-pushed the copilot/fix-checkptr-race-issue branch from 9cd6261 to bcc8f55 Compare March 18, 2026 23:07
@kpango
Copy link
Copy Markdown
Owner

kpango commented Mar 18, 2026

/diff

@github-actions
Copy link
Copy Markdown

Benchmark Comparison (vs main)

Failed to generate benchstat output. Check GitHub Actions logs for details.

@kpango
Copy link
Copy Markdown
Owner

kpango commented Mar 18, 2026

@copilot can you please fix /diff chatbot failure?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 18, 2026

@copilot can you please fix /diff chatbot failure?

The PR diff was noisy due to unrelated changes (Makefile, go.mod, go.sum, counter refactoring) that were accidentally bundled into the commit. Cleaned up in 874e7c5 — the diff now only contains the minimal map.go changes needed to fix the checkptr issue.

@kpango kpango force-pushed the copilot/fix-checkptr-race-issue branch from 874e7c5 to bcc8f55 Compare March 23, 2026 03:15
…per-entry typed expunged field

Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango force-pushed the copilot/fix-checkptr-race-issue branch from bcc8f55 to 3ae43e4 Compare March 23, 2026 03:15
@kpango kpango merged commit afd90df into main Mar 23, 2026
3 of 4 checks passed
@kpango kpango deleted the copilot/fix-checkptr-race-issue branch March 23, 2026 03:17
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.

checkptr error with -race flag since v2.1.5: converted pointer straddles multiple allocations

2 participants