Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

direct memory allocation error in PooledByteBufAllocator when setting directMemoryCacheAlignment to "1" #11101

Closed
alalag1 opened this issue Mar 19, 2021 · 1 comment · Fixed by #11106
Closed

direct memory allocation error in PooledByteBufAllocator when setting directMemoryCacheAlignment to "1" #11101

alalag1 opened this issue Mar 19, 2021 · 1 comment · Fixed by #11106
Milestone
4.1.61.Final

Comments

@alalag1
Copy link

alalag1 commented Mar 19, 2021

Define a PooledByteBufAllocator and set ·directMemoryCacheAlignment· to "1" and try to allocate some ByteBuf like

PooledByteBufAllocator pooledByteBufAllocator = new PooledByteBufAllocator(
        PlatformDependent.directBufferPreferred(),
        PooledByteBufAllocator.defaultNumHeapArena(),
        PooledByteBufAllocator.defaultNumDirectArena(),
        PooledByteBufAllocator.defaultPageSize(),
        11,
        PooledByteBufAllocator.defaultSmallCacheSize(),
        64,
        PooledByteBufAllocator.defaultUseCacheForAllThreads(),
        1
);

ByteBuf buf1 = pooledByteBufAllocator.directBuffer(16384);
for (int i = 0; i < 16384; i++) {
    buf1.writeByte(1);
}
ByteBuf buf2 = pooledByteBufAllocator.directBuffer(65536);
buf2.writeByte(2);
System.out.println("last byte of buf1: " + buf1.getByte(buf1.capacity() - 1));

Expected behavior

last byte of the buf1: 1

Actual behavior

last byte of the buf1: 2

Steps to reproduce

PooledByteBufAllocator pooledByteBufAllocator = new PooledByteBufAllocator(
        PlatformDependent.directBufferPreferred(),
        PooledByteBufAllocator.defaultNumHeapArena(),
        PooledByteBufAllocator.defaultNumDirectArena(),
        PooledByteBufAllocator.defaultPageSize(),
        11,
        PooledByteBufAllocator.defaultSmallCacheSize(),
        64,
        PooledByteBufAllocator.defaultUseCacheForAllThreads(),
        1
);

ByteBuf buf1 = pooledByteBufAllocator.directBuffer(16384);
ByteBuf buf2 = pooledByteBufAllocator.directBuffer(65536);
System.out.println("memory address of buf1: [" + buf1.memoryAddress() + ", " + (buf1.memoryAddress() + buf1.capacity()) + ")");
System.out.println("memory address of buf2: [" + buf2.memoryAddress() + ", " + (buf2.memoryAddress() + buf2.capacity()) + ")");

Got 2 overlapped ranges(on my pc)
memory address of buf1: [2047716884545, 2047716900929)
memory address of buf2: [2047716900928, 2047716966464)

Netty version

4.1.60.Final

JVM version (e.g. java -version)

1.8.0_271

OS version (e.g. uname -a)

win10
@chrisvest
Copy link
Contributor

@alalag1 Oof! Nice catch. We'll take a look.

@chrisvest chrisvest mentioned this issue Mar 22, 2021
Merged
chrisvest added a commit to chrisvest/netty that referenced this issue Mar 22, 2021
@chrisvest
Fix alignment handling for pooled direct buffers
2ff5b90 
Motivation:
Alignment handling was broken, and basically turned into a fixed offset into each allocation address regardless of its initial value, instead of ensuring that the allocated address is either aligned or bumped to the nearest alignment offset.
The brokenness of the alignment handling extended so far, that overlapping ByteBuf instances could even be created, as was seen in netty#11101.

Modification:
Instead of fixing the per-allocation pointer bump, we now ensure that 1) the minimum page size is a whole multiple of the alignment, and 2) the reference memory for each chunk is bumped to the nearest aligned address, and finally 3) ensured that the reservations are whole multiples of the alignment, thus ensuring that the next allocation automatically occurs from an aligned address.

Incidentally, (3) above comes for free because the reservations are in whole pages, and in (1) we ensured that pages are sized in whole multiples of the alignment.

In order to ensure that the memory for a chunk is aligned, we introduce some new PlatformDependent infrastructure.
The PlatformDependent.alignDirectBuffer will produce a slice of the given buffer, and the slice will have an address that is aligned.
This method is plainly available on ByteBuffer in Java 9 onwards, but for pre-9 we have to use Unsafe, which means it can fail and might not be available on all platforms.
Attempts to create a PooledByteBufAllocator that uses alignment, when this is not supported, will throw an exception.
Luckily, I think use of aligned allocations are rare.

Result:
Aligned pooled byte bufs now work correctly, and never have any overlap.

Fixes netty#11101
chrisvest added a commit that referenced this issue Mar 23, 2021
@chrisvest
Fix alignment handling for pooled direct buffers (#11106)
2071086 
Motivation:
Alignment handling was broken, and basically turned into a fixed offset into each allocation address regardless of its initial value, instead of ensuring that the allocated address is either aligned or bumped to the nearest alignment offset.
The brokenness of the alignment handling extended so far, that overlapping ByteBuf instances could even be created, as was seen in #11101.

Modification:
Instead of fixing the per-allocation pointer bump, we now ensure that 1) the minimum page size is a whole multiple of the alignment, and 2) the reference memory for each chunk is bumped to the nearest aligned address, and finally 3) ensured that the reservations are whole multiples of the alignment, thus ensuring that the next allocation automatically occurs from an aligned address.

Incidentally, (3) above comes for free because the reservations are in whole pages, and in (1) we ensured that pages are sized in whole multiples of the alignment.

In order to ensure that the memory for a chunk is aligned, we introduce some new PlatformDependent infrastructure.
The PlatformDependent.alignDirectBuffer will produce a slice of the given buffer, and the slice will have an address that is aligned.
This method is plainly available on ByteBuffer in Java 9 onwards, but for pre-9 we have to use Unsafe, which means it can fail and might not be available on all platforms.
Attempts to create a PooledByteBufAllocator that uses alignment, when this is not supported, will throw an exception.
Luckily, I think use of aligned allocations are rare.

Result:
Aligned pooled byte bufs now work correctly, and never have any overlap.

Fixes #11101
chrisvest added a commit that referenced this issue Mar 23, 2021
@chrisvest
Fix alignment handling for pooled direct buffers (#11106)
9ba653c 
Motivation:
Alignment handling was broken, and basically turned into a fixed offset into each allocation address regardless of its initial value, instead of ensuring that the allocated address is either aligned or bumped to the nearest alignment offset.
The brokenness of the alignment handling extended so far, that overlapping ByteBuf instances could even be created, as was seen in #11101.

Modification:
Instead of fixing the per-allocation pointer bump, we now ensure that 1) the minimum page size is a whole multiple of the alignment, and 2) the reference memory for each chunk is bumped to the nearest aligned address, and finally 3) ensured that the reservations are whole multiples of the alignment, thus ensuring that the next allocation automatically occurs from an aligned address.

Incidentally, (3) above comes for free because the reservations are in whole pages, and in (1) we ensured that pages are sized in whole multiples of the alignment.

In order to ensure that the memory for a chunk is aligned, we introduce some new PlatformDependent infrastructure.
The PlatformDependent.alignDirectBuffer will produce a slice of the given buffer, and the slice will have an address that is aligned.
This method is plainly available on ByteBuffer in Java 9 onwards, but for pre-9 we have to use Unsafe, which means it can fail and might not be available on all platforms.
Attempts to create a PooledByteBufAllocator that uses alignment, when this is not supported, will throw an exception.
Luckily, I think use of aligned allocations are rare.

Result:
Aligned pooled byte bufs now work correctly, and never have any overlap.

Fixes #11101
@normanmaurer normanmaurer added this to the 4.1.61.Final milestone Mar 30, 2021
raidyue pushed a commit to raidyue/netty that referenced this issue Jul 8, 2022
@chrisvest
Fix alignment handling for pooled direct buffers (netty#11106)
866613f 
Motivation:
Alignment handling was broken, and basically turned into a fixed offset into each allocation address regardless of its initial value, instead of ensuring that the allocated address is either aligned or bumped to the nearest alignment offset.
The brokenness of the alignment handling extended so far, that overlapping ByteBuf instances could even be created, as was seen in netty#11101.

Modification:
Instead of fixing the per-allocation pointer bump, we now ensure that 1) the minimum page size is a whole multiple of the alignment, and 2) the reference memory for each chunk is bumped to the nearest aligned address, and finally 3) ensured that the reservations are whole multiples of the alignment, thus ensuring that the next allocation automatically occurs from an aligned address.

Incidentally, (3) above comes for free because the reservations are in whole pages, and in (1) we ensured that pages are sized in whole multiples of the alignment.

In order to ensure that the memory for a chunk is aligned, we introduce some new PlatformDependent infrastructure.
The PlatformDependent.alignDirectBuffer will produce a slice of the given buffer, and the slice will have an address that is aligned.
This method is plainly available on ByteBuffer in Java 9 onwards, but for pre-9 we have to use Unsafe, which means it can fail and might not be available on all platforms.
Attempts to create a PooledByteBufAllocator that uses alignment, when this is not supported, will throw an exception.
Luckily, I think use of aligned allocations are rare.

Result:
Aligned pooled byte bufs now work correctly, and never have any overlap.

Fixes netty#11101
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.1.61.Final
Development

Successfully merging a pull request may close this issue.

Fix alignment handling for pooled direct buffers chrisvest/netty
3 participants
@chrisvest @normanmaurer @alalag1