Skip to content

Commit

Permalink
Fix inconsistent synchronization of PoolSubpage.doNotDestroy (#12775)
Browse files Browse the repository at this point in the history
Motivation:
This field is modified under lock, and thus should not be accessed (toString excepted) without locking.

Modification:
Move an assert check on PoolSubpage.doNotDestroy into a nearby critical region.

Also encapsulate the findSubpagePoolHead logic better; PoolArena now always use this method instead of that one place that was accessing the array directly.
This should not have any influence on the logic.

Result:
Assert in PoolChunk is no longer racy.
  • Loading branch information
chrisvest committed Sep 7, 2022
1 parent 195a370 commit 341e4f1
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion buffer/src/main/java/io/netty/buffer/PoolArena.java
Expand Up @@ -157,7 +157,7 @@ private void tcacheAllocateSmall(PoolThreadCache cache, PooledByteBuf<T> buf, fi
* Synchronize on the head. This is needed as {@link PoolChunk#allocateSubpage(int)} and
* {@link PoolChunk#free(long)} may modify the doubly linked list as well.
*/
final PoolSubpage<T> head = smallSubpagePools[sizeIdx];
final PoolSubpage<T> head = findSubpagePoolHead(sizeIdx);
final boolean needsNormalAllocation;
head.lock();
try {
Expand Down
2 changes: 1 addition & 1 deletion buffer/src/main/java/io/netty/buffer/PoolChunk.java
Expand Up @@ -479,12 +479,12 @@ void free(long handle, int normCapacity, ByteBuffer nioBuffer) {

int sIdx = runOffset(handle);
PoolSubpage<T> subpage = subpages[sIdx];
assert subpage != null && subpage.doNotDestroy;

// Obtain the head of the PoolSubPage pool that is owned by the PoolArena and synchronize on it.
// This is need as we may add it back and so alter the linked-list structure.
head.lock();
try {
assert subpage != null && subpage.doNotDestroy;
if (subpage.free(head, bitmapIdx(handle))) {
//the subpage is still used, do not free it
return;
Expand Down

0 comments on commit 341e4f1

Please sign in to comment.