Skip to content

Adaptive: Fix concurrency issue in adaptive allocator#16767

Merged
chrisvest merged 5 commits into
netty:4.2from
chrisvest:4.2-adaptive-fix
May 8, 2026
Merged

Adaptive: Fix concurrency issue in adaptive allocator#16767
chrisvest merged 5 commits into
netty:4.2from
chrisvest:4.2-adaptive-fix

Conversation

@chrisvest
Copy link
Copy Markdown
Member

Motivation:
We have a few code paths where Chunk.remainingCapacity() is called without regard for exclusive access to the given chunk. The BuddyChunk implementation unfortunately drained the freeList in this call, leading to racy modifications of the buddies array, that in turn can manifest as unpredictable bugs.

Modification:
Make the BuddyChunk.remainingCapacity non-modifying. Instead of draining the freeList, we sum up the capacity held by it and add it to the capacity accounted for in the Chunk super class.

This operation needs a weakPeekReduce method on the MpscIntQueue. This method performs a reduction operation on a given range of entries, but without removing entries or incrementing the consumer index.

A regression test is added, which successfully captured the issue before it was fixed.

Also did a few test cleanups.

Result:
No more corruptions in the buddies array.

This fixes the rare occurrences of assertion errors like the following:

java.lang.AssertionError
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.allocate(AdaptivePoolingAllocator.java:916)
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.tryAllocate(AdaptivePoolingAllocator.java:854)
	at io.netty.buffer.AdaptivePoolingAllocator$MagazineGroup.allocate(AdaptivePoolingAllocator.java:421)
	at io.netty.buffer.AdaptivePoolingAllocator.allocate(AdaptivePoolingAllocator.java:271)

Motivation:
We have a few code paths where `Chunk.remainingCapacity()` is called without regard for exclusive access to the given chunk.
The `BuddyChunk` implementation unfortunately drained the `freeList` in this call, leading to racy modifications of the `buddies` array, that in turn can manifest as unpredictable bugs.

Modification:
Make the `BuddyChunk.remainingCapacity` non-modifying.
Instead of draining the `freeList`, we sum up the capacity held by it and add it to the capacity accounted for in the `Chunk` super class.

This operation needs a `weakPeekReduce` method on the `MpscIntQueue`.
This method performs a reduction operation on a given range of entries, but without removing entries or incrementing the consumer index.

A regression test is added, which successfully captured the issue before it was fixed.

Also did a few test cleanups.

Result:
No more corruptions in the buddies array.

This fixes the rare occurrences of assertion errors like the following:

```
java.lang.AssertionError
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.allocate(AdaptivePoolingAllocator.java:916)
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.tryAllocate(AdaptivePoolingAllocator.java:854)
	at io.netty.buffer.AdaptivePoolingAllocator$MagazineGroup.allocate(AdaptivePoolingAllocator.java:421)
	at io.netty.buffer.AdaptivePoolingAllocator.allocate(AdaptivePoolingAllocator.java:271)
```
@chrisvest chrisvest added this to the 4.2.14.Final milestone May 7, 2026
@chrisvest chrisvest added needs-cherry-pick-4.1 This PR should be cherry-picked to 4.1 once merged. needs-cherry-pick-5.0 This PR should be cherry-picked to 5.0 once merged. labels May 7, 2026
@chrisvest chrisvest force-pushed the 4.2-adaptive-fix branch from ca1f910 to 62a3fd2 Compare May 8, 2026 02:36
Comment thread common/src/main/java/io/netty/util/concurrent/MpscIntQueue.java Outdated
Comment thread buffer/src/test/java/io/netty/buffer/AdaptiveByteBufAllocatorTest.java Outdated
Comment thread buffer/src/test/java/io/netty/buffer/AdaptiveByteBufAllocatorGrowthTest.java Outdated
@chrisvest
Copy link
Copy Markdown
Member Author

@normanmaurer @franz1981 comments addressed

@chrisvest chrisvest merged commit bd866c3 into netty:4.2 May 8, 2026
18 of 19 checks passed
@chrisvest chrisvest deleted the 4.2-adaptive-fix branch May 8, 2026 19:09
@netty-project-bot
Copy link
Copy Markdown
Contributor

Could not create auto-port PR.
Got conflicts when cherry-picking onto 4.1.

@netty-project-bot
Copy link
Copy Markdown
Contributor

Auto-port PR for 5.0: #16777

@github-actions github-actions Bot removed the needs-cherry-pick-5.0 This PR should be cherry-picked to 5.0 once merged. label May 8, 2026
@chrisvest
Copy link
Copy Markdown
Member Author

4.1 PR: #16778

@chrisvest chrisvest removed the needs-cherry-pick-4.1 This PR should be cherry-picked to 4.1 once merged. label May 8, 2026
chrisvest added a commit that referenced this pull request May 8, 2026
…16777)

Auto-port of #16767 to 5.0
Cherry-picked commit: bd866c3

---
Motivation:
We have a few code paths where `Chunk.remainingCapacity()` is called
without regard for exclusive access to the given chunk. The `BuddyChunk`
implementation unfortunately drained the `freeList` in this call,
leading to racy modifications of the `buddies` array, that in turn can
manifest as unpredictable bugs.

Modification:
Make the `BuddyChunk.remainingCapacity` non-modifying. Instead of
draining the `freeList`, we sum up the capacity held by it and add it to
the capacity accounted for in the `Chunk` super class.

This operation needs a `weakPeekReduce` method on the `MpscIntQueue`.
This method performs a reduction operation on a given range of entries,
but without removing entries or incrementing the consumer index.

A regression test is added, which successfully captured the issue before
it was fixed.

Also did a few test cleanups.

Result:
No more corruptions in the buddies array.

This fixes the rare occurrences of assertion errors like the following:

```
java.lang.AssertionError
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.allocate(AdaptivePoolingAllocator.java:916)
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.tryAllocate(AdaptivePoolingAllocator.java:854)
	at io.netty.buffer.AdaptivePoolingAllocator$MagazineGroup.allocate(AdaptivePoolingAllocator.java:421)
	at io.netty.buffer.AdaptivePoolingAllocator.allocate(AdaptivePoolingAllocator.java:271)
```

Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
chrisvest added a commit that referenced this pull request May 9, 2026
Motivation:
We have a few code paths where `Chunk.remainingCapacity()` is called
without regard for exclusive access to the given chunk. The `BuddyChunk`
implementation unfortunately drained the `freeList` in this call,
leading to racy modifications of the `buddies` array, that in turn can
manifest as unpredictable bugs.

Modification:
Make the `BuddyChunk.remainingCapacity` non-modifying. Instead of
draining the `freeList`, we sum up the capacity held by it and add it to
the capacity accounted for in the `Chunk` super class.

This operation needs a `weakPeekReduce` method on the `MpscIntQueue`.
This method performs a reduction operation on a given range of entries,
but without removing entries or incrementing the consumer index.

A regression test is added, which successfully captured the issue before
it was fixed.

Also did a few test cleanups.

Result:
No more corruptions in the buddies array.

This fixes the rare occurrences of assertion errors like the following:

```
java.lang.AssertionError
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.allocate(AdaptivePoolingAllocator.java:916)
	at io.netty.buffer.AdaptivePoolingAllocator$Magazine.tryAllocate(AdaptivePoolingAllocator.java:854)
	at io.netty.buffer.AdaptivePoolingAllocator$MagazineGroup.allocate(AdaptivePoolingAllocator.java:421)
	at io.netty.buffer.AdaptivePoolingAllocator.allocate(AdaptivePoolingAllocator.java:271)
```

(cherry picked from commit bd866c3)
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.

4 participants