Skip to content

fix: data race in ehash due to unsynchronized lazy init of Map.typ#4

Open
stuchl4n3k wants to merge 1 commit intolrita:masterfrom
stuchl4n3k:master
Open

fix: data race in ehash due to unsynchronized lazy init of Map.typ#4
stuchl4n3k wants to merge 1 commit intolrita:masterfrom
stuchl4n3k:master

Conversation

@stuchl4n3k
Copy link
Copy Markdown

ehash uses double-checked locking to lazily initialize m.typ on first
use. The outer check reads m.typ without synchronization, while the
inner write happens under m.lock. This violates Go's memory model —
a concurrent reader can observe a partially written pointer.

Replace the bare *rtype field with atomic.Pointer[rtype] and use
Load/Store in ehash so the fast path is a single atomic load and the
write under the lock is correctly published to other goroutines.

lock sync.Mutex
inode unsafe.Pointer // *inode2
typ *rtype
typ atomic.Pointer[rtype]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you use unsafe.Pointer here, or upgrade the minimum dependency in go.mod to a version that supports this type?

m.lock.Unlock()
}

typ := m.typ.Load()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We can move this to the beginning of the function so it only loads once for the most common scenarios. Meanwhile, let's refine the double-check logic to make it works fine.

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.

2 participants