(3)-masquerade: the block-handover races, and honest concurrent oracles - #1699
Draft
daniel-noland wants to merge 3 commits into
Draft
(3)-masquerade: the block-handover races, and honest concurrent oracles#1699daniel-noland wants to merge 3 commits into
daniel-noland wants to merge 3 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This was referenced Aug 6, 2026
daniel-noland
changed the title
daniel-noland
marked this pull request as draft
August 6, 2026 02:32
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 6, 2026 04:35
58bf053 to
ceb138c
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 04:35
e862f7f to
6cc4f54
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect bitmap mutation in the masquerade port allocator by making single-port updates correct in both directions (reserve and free) and by strengthening the API to accept a bool (used/free) rather than an integer.
Changes:
- Correct
Bitmap256::set_bitmap_valueso clearing a bit actually clears it and “already used/free” checks behave correctly for all bit positions. - Update reserve/deallocate helpers to use the new
bool-based API. - Add focused unit and integration-style tests to ensure freed ports become available again and double-reservation is rejected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| nat/src/masquerade/apalloc/port_alloc.rs | Fixes per-port bitmap updates (reserve/free) and adds unit tests for freeing and reservation guards. |
| nat/src/masquerade/apalloc/pool_fuzz.rs | Adds a regression test ensuring a single freed port is reusable even while neighboring allocations keep the block alive. |
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 6, 2026 21:25
ceb138c to
a17cbd1
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 21:25
74bbea2 to
3e63b8e
Compare
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
changed the title
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 6, 2026 21:47
a17cbd1 to
cf59591
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 21:47
3e63b8e to
2b807d5
Compare
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 22:29
2b807d5 to
12afe1d
Compare
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 22:33
12afe1d to
f86b776
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 6, 2026 22:33
a5c7af6 to
c8219f9
Compare
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 6, 2026 22:49
c8219f9 to
51bc9db
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 22:49
f86b776 to
12c53b7
Compare
daniel-noland
added a commit
that referenced
this pull request
Aug 6, 2026
…lock CI caught this before we did: sanitize/fuzz/thread on #1699 ran for six hours and was killed with the nat test binary still alive, one test short of the suite. Locally the same test wedges within about ten runs, and 3000 runs pass with this change. The pool keeps weak references to the addresses in use; the strong ones belong to the port blocks handed out from each address. Upgrading one while holding the pool lock is safe only until the last flow on that address ends somewhere else, at which point the upgrade here is the only strong reference left and letting it go runs AllocatedIp::drop on this thread. That drop asks the pool for its write lock, which this thread is already holding. The core stops, for good. Three places did it, all reached by IpAllocator::allocate, which is the path every new flow takes: * cleanup, which upgrades each entry to see whether it still resolves, under the write lock. This is the one that hangs: it runs on every allocation, and the temporary upgrade is dropped immediately. * reuse_allocated_ip, under the read lock, for each address it passes over. * reserve_from_pool, under the write lock, for each address that is not the one being carried over. Each now keeps what it upgraded until the guard is gone and releases it after. Confirmed by intervention rather than by reading: fixing only reuse_allocated_ip left it hanging at iteration 25, and fixing cleanup took it to 3000 clean. Pre-existing: cleanup is unchanged from main, and the test that exposes it is on main too. It hid because the window is small and needs a flow ending on one thread while another allocates. sanitize/fuzz/thread found it because it runs the whole suite on real threads for long enough. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 6, 2026 23:51
49725e7 to
1e0f577
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
2 times, most recently
from
August 7, 2026 01:07
29a8636 to
7026f44
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
2 times, most recently
from
August 7, 2026 01:30
5010f75 to
f60f8f0
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 7, 2026 01:30
7026f44 to
67f2ea5
Compare
The pool-level half of the freeing fix, which now lands at the bottom of the stack so that everything built on top can rely on it. Dropping every allocation at once frees whole blocks, and a block is rebuilt from scratch whatever its bitmap said, so a test that does that passes whether or not an individual port is ever returned. Only a port given back while its block stays alive shows whether freeing works, and that is the ordinary case: one flow ending while its neighbours carry on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
The ReserveExisting op reserved a survivor's pair on a published allocator and asserted only that the error was not InternalIssue. Two problems. It matched on Err in a let-chain, so a reservation that succeeded produced a temporary that dropped at the end of the statement. That releases the port the published generation is holding for the survivor. Had a bug ever let a pair be reserved twice, the op would not have caught it, and would have corrupted the state the other two oracles rest on while failing to. And succeeding is itself the interesting outcome. The writer re-reserves every survivor before publishing, so where a pair was carried over, a second reservation has to be refused: the same rule as for allocation, on the path a config change actually takes. That is now asserted. Where it was not carried over the pair is genuinely free, and a reservation that succeeds is recorded and held for the length of the run like any other allocation, rather than being handed back while the other threads work. The same suite accepted a second kind of vacuous success. Published::build accepted any carry-over failure short of InternalIssue, on the comment that the address may no longer be served or another survivor may hold the pair. Neither can happen: every generation is built from the same specs, so the address is still served; the survivors are distinct pairs; and the allocator is fresh, so nothing else holds them. A regression that made carry-over fail would have gone unseen, and every property about carried pairs would have passed vacuously over generations that quietly carried nothing. A survivor that fails to carry is a failure now. Being strict there also settles what ReserveExisting is: every survivor is always carried, so in a correct allocator the reservation is always refused, and the success arm is an oracle for a double-reservation bug rather than a covered path. Its comment now says so. The module doc claimed reserving concurrently with allocating is what would show the standing find_block_for_port FIXME, the block released between the CAS and the lookup. It is not, and the doc now explains why: reservations target survivors, and a survivor's block is pinned for the whole generation by the reservation Published holds, so it cannot disappear mid-lookup. Reaching that interleaving takes generations whose specs differ, so that a pair stops being carried and its block can empty while another thread reserves it. That is the suite's next extension, recorded rather than implied to exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
A block is given back in two steps a reader can fall between: the free flag is stored true, and the weak entry in the list of allocated blocks expires when the last Arc to it goes. A reservation reads the flag and then searches that list, so it could find the block neither free nor allocated and returned InternalIssue -- the allocator declaring its own bookkeeping broken -- for a moment in which nothing is wrong. The lookup now starts over instead. The port the reservation wants is free by then, so the next attempt claims the block and allocates it. The loop is bounded because a thread that keeps allocating and releasing this same block could otherwise hold a reserver in the window indefinitely; on exhaustion the answer is a failed reservation, which costs one flow, rather than InternalIssue, which the caller reads as the allocator being unfit and which the concurrency model treats as fatal. This carried a FIXME wondering whether the window was reachable and noting it had not been seen in shuttle. It is reachable, and shuttle finds it in one execution: the test added here allocates a pair, races a reservation of it against dropping its only holder, and accepts either legitimate answer -- refused while held, granted once released. It fails on the first schedule without the retry. Reaching it needs neither a config change nor generations whose specs differ, which is what the module doc claimed; one pool and one generation will do. Production meets it through the late flow nf.rs handles, where a packet that allocated from the previous allocator installs its flow after a new one was published: that pair is not among the writer's pinned survivors, so the block behind it may be emptying as it re-reserves. The bug predates this stack. It is fixed here because this is the PR that makes the concurrent oracle honest, and the claim it corrects is one this PR's own documentation made. A third instance of the same shape, found by review of this PR and fixed here with it. Tidying a dead entry out of the list of allocated blocks looks the entry up and drops it under separate locks, so another task can claim the freed block and list it at that index in between -- and the drop then deletes an entry for a block in use. Nothing is handed out twice, so this is availability rather than isolation: the orphaned block stays claimed by its holder while the allocator no longer knows of it, so reservations into it are refused and its free ports stop counting towards the address having room. Re-checking under the write lock is the answer here, rather than a retry, since the caller has a lock to take anyway. `search_for_block` upgraded twice for the same reason -- once to test the block, once to return it -- and could report a block absent because it died between the two. It keeps the first upgrade now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/port-bitmap-free
branch
from
August 7, 2026 01:50
f60f8f0 to
6a43add
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/portfw-reservation-fixes
branch
from
August 7, 2026 01:50
67f2ea5 to
3014ecf
Compare
daniel-noland
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
AI assisted, not yet ready for external review by other humans.
Please do not spend review time on this yet. It is pushed to run CI and to keep
the stack visible, not to attract review. The
dont-mergelabel stays on untilthat changes.
The
set_bitmap_valuefix this PR used to carry has moved to the bottom of the stack, into#1696. It had to: the concurrent model checker cannot assert that a pair already held is refused a
second time until that fix is in, and a suite that accepts such a success certifies the defect
instead of catching it. With it at the bottom, no PR in the stack ever ships a live
duplicate-reservation bug.
What is left here is the testing that fix makes possible.
The concurrent suite now guards this fix
The model checker's reservation oracle moved here from further up the stack, because here is where
it can hold. It asserts that a second reservation of a pair a carried-over flow still holds is
refused -- which is exactly what the duplicate-reservation guard above makes true, and what was
false before it. Under the previous arrangement the suite accepted such a success and dropped it,
so it passed while certifying the very defect this PR fixes.
Mutation-verified against this PR's own change: revert the
set_bitmap_valueguard andstress_test_config_changefails.Published::buildalso stopped accepting carry-over failures. Every generation is built from thesame specs with a fresh allocator and distinct survivor pairs, so nothing may refuse one; tolerating
it let the model publish generations that carried nothing, and every property about carried pairs
passed vacuously over them.
A race the model could not reach, found by review
The suite's own documentation claimed the reservation/release window in
find_block_for_portneeded generations whose specs differ to reach. That was wrong, and the claim was made by this
PR. One pool and one generation reach it: allocate a pair, then race a reservation of that pair
against dropping its only holder. Shuttle finds the schedule in a single execution.
A block is given back in two steps a reader can fall between -- the free flag is stored
true,and the weak entry in the list of allocated blocks expires with the last
Arc. A reservationreads the flag and then searches the list, so it could see the block as neither, and returned
InternalIssue: the allocator declaring its own bookkeeping broken for a moment in which nothingis wrong. The lookup now starts over, bounded, and answers a failed reservation if every attempt
lands in the window.
Production reaches this through the late flow
nf.rsalready handles, where a packet thatallocated from the previous allocator installs its flow after a new one is published. That pair
is not among the writer's pinned survivors, so the block behind it can be emptying as it
re-reserves. The cost was a spuriously invalidated flow during a config change.
The defect predates this stack -- the code carried a
FIXMEwondering whether the window wasreachable and noting it had not been seen under shuttle. It is fixed here rather than lower
because this is the PR that makes the concurrent oracle honest, and the claim it corrects is one
this PR made. The regression test fails on the first schedule without the fix.
What the suite still cannot reach on its own is unchanged and now stated correctly: its
reservations target survivors, whose blocks are pinned for the generation. Widening
ReserveExistingto unpinned pairs is the extension that would close it.A third instance of the same shape
Review of the whole stack found the same two-step handover a third time, and it is fixed here
alongside the other two. Tidying a dead entry out of the list of allocated blocks looks the entry
up and drops it under separate locks, so another thread can claim the freed block and list it at
that index in between; the drop then deletes an entry for a block that is in use.
Nothing is handed out twice, so this is availability rather than isolation — the orphaned block
stays claimed by its holder while the allocator no longer knows of it, so reservations into it are
refused and its free ports stop counting towards the address having room. Shuttle reproduces it and
the symptom is exactly that:
PortReservationFailedon a block with 255 ports free.The answer here is to re-check under the write lock rather than to retry, since the caller has a
lock to take anyway.
search_for_blockhad the same double-upgrade shape and now keeps its firstone.
One honest note on the regression test: the interleaving is narrow and
stressallows sixteeniterations over three schedulers, so reverting the fix is caught in about two runs in three rather
than every run. It never fails with the fix in place, so it costs nothing in CI. Making the
contending task poised before the block is freed was tried and measured worse — four runs in
twelve against eight — so the simpler version stayed.
Stack
Merge bottom to top. Each PR is based on the one above it in this list.
#1697 was folded into #1696 and closed; its one commit belonged next to the other pool-table work.
Every commit in the stack builds and passes
cargo nextest runandcargo clippy --all-targetson its own.