Skip to content

server/item/inventory: fix data race between a slot change and SlotFunc or Close - #1417

Open
schphe wants to merge 4 commits into
df-mc:masterfrom
oriumgames:fix/inventory-slotfunc-race
Open

server/item/inventory: fix data race between a slot change and SlotFunc or Close#1417
schphe wants to merge 4 commits into
df-mc:masterfrom
oriumgames:fix/inventory-slotfunc-race

Conversation

@schphe

@schphe schphe commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What happens and why

setItem returns a closure that its callers run after the inventory has been
unlocked, so that the function called on a slot change does not run while the
inventory is locked. That closure read inv.f when it ran rather than when it
was built, while SlotFunc and Close write inv.f under the lock, so a slot
being written concurrently with either was a data race on a type documented as
safe for concurrent use.

The function is now read while the lock is still held. A change already in
progress therefore calls the function that was installed when it started,
which is the same guarantee the lock gives every other field.

Verification

Regression test reports WARNING: DATA RACE without the fix, under go test -race. Note the repository's CI does not run with the race detector, so this test only earns its place if run that way.

@schphe
schphe force-pushed the fix/inventory-slotfunc-race branch 2 times, most recently from 50285a7 to 0b0c40f Compare August 12, 2026 01:36
Comment thread server/item/inventory/inventory.go Outdated
@schphe
schphe force-pushed the fix/inventory-slotfunc-race branch from a8e09b7 to efd813f Compare August 12, 2026 05:08
@schphe
schphe force-pushed the fix/inventory-slotfunc-race branch from 8503f9a to b788bc8 Compare August 18, 2026 00:45
schphe added 4 commits August 20, 2026 02:47
…nc or Close

setItem returns a closure that its callers run after the inventory has been
unlocked, so that the function called on a slot change does not run while the
inventory is locked. That closure read inv.f when it ran rather than when it
was built, while SlotFunc and Close write inv.f under the lock, so a slot
being written concurrently with either was a data race on a type documented as
safe for concurrent use.

The function is now read while the lock is still held. A change already in
progress therefore calls the function that was installed when it started,
which is the same guarantee the lock gives every other field.
@schphe
schphe force-pushed the fix/inventory-slotfunc-race branch from b788bc8 to 9c5c7f9 Compare August 20, 2026 07:51
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