floatdrop / floatdrop/di

Wrap in a descendant can race an ancestor's Override and leave two live values for a key

Open
#52 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
3
Forks
0
Avg merge
16m
Merged PRs (30d)
46

Description

A descendant scope's `Wrap` and an ancestor scope's `Override` of the same registration can race so that the `Override` is accepted under a live wrapper. The ancestor then serves the new value, while the descendant's wrapper still serves a value built from the registration it replaced. That is two live values for one key, which the wrapper guard exists to prevent, and nothing reports it.

## The interleaving

`Wrap` binds to its target in two steps:

1. `Wrap` looks up the registration that serves the key: `current` in its own scope, else `lookup` from the parent.
2. `register`'s init calls `inner.addWrapper(b)`.

An ancestor's `freeze` rejects an `Override` while the registration still has wrappers (`guard.against`). If that check runs between steps 1 and 2 on another goroutine, it finds no wrapper, commits the replacement, and step 2 then marks a registration nothing else can reach.

The same shape applies to a link an `Override` retires: a `Wrap` that looked the link up before the commit can add itself after `release` has already dropped the link's mark on what it wraps.

## Reachability

It needs registrations running concurrently, on different goroutines, in two scopes of the same container, on the same key, at the same instant. Programs normally register at startup on one goroutine, and tests override before resolving, so in practice it takes configuration racing itself. It is still a silent invariant violation.

## Not new

0.16.1 has the same window: `Wrap` read its target and later stored the single `wrappedBy` pointer. PR #51 keeps the behaviour and documents the window in CLAUDE.md as accepted.

## Proposed fix

A Dekker pair, like the drain seal in #51 (`announce`/`seal`):

- The ancestor's `freeze` marks the registration it is about to replace as *being replaced*, then checks its wrappers.
- `Wrap` records its mark, then checks the flag, and rejects the registration if it is set, with a configuration panic naming both sites.

Both sides cannot miss each other: if the `freeze` saw no wrapper, `Wrap`'s check comes after the flag was set.

The fiddly part is a batch that sets the flag and is then rejected for some other reason. The flag has to be cleared again, or the registration stays unwrappable. Setting the flags only after the batch has passed every other check, and clearing any set by an earlier entry if a later flag check fails, should keep `freeze` transactional.

## Tests

A deterministic internal test in package `di`, like `TestSealDecidesAClaimedStart`: perform step 1 of a `Wrap` by hand, commit the ancestor's `Override`, then perform step 2, and expect a rejection rather than silent acceptance. Then the retired-link variant. Also a `FuzzMachineConcurrent` shape that registers a wrapper in a child while the root overrides the key.

## Docs to correct

CLAUDE.md, in the "Wrap binds at registration" paragraph, currently says closing this "would mean ordering two scopes' commits against each other, which no two state mutexes ever are". That overstates it, since the Dekker pair above needs no lock ordering. Update or remove that sentence with the fix.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read the Wrap and Override paths, then inspect CLAUDE.md's “Wrap binds at registration” paragraph. Reproduce the deterministic interleaving in a package di test like TestSealDecidesAClaimedStart, cover the retired-link variant, and consider the FuzzMachineConcurrent shape described in the issue. Done means the races reject invalid registrations, preserve transactional behavior, and the outdated documentation is corrected.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.