Skip to content

Commit 7937553

Browse files
j-bahrchrisvest
j-bahr
and
chrisvest
authored
Enforce io.netty.maxDirectMemory accounting on all Java versions (#16489)
**Motivation**: Netty selects a Cleaner implementation based on the Java version and whether `sun.misc.Unsafe` is available. The selection matrix is: ``` Java version | Unsafe available | Cleaner selected ───────────────|──────────────────|───────────────────── 6-8 | Yes | DirectCleaner 9-23 | Yes | DirectCleaner 24 | Yes (warnings) | DirectCleaner 24 | No, native access| CleanerJava24Linker 25+ | No, native access| CleanerJava24Linker 25+ | No | CleanerJava25 ``` Before this change, direct memory accounting (`incrementMemoryCounter` / `decrementMemoryCounter`) was coupled to `USE_DIRECT_BUFFER_NO_CLEANER`, which was only true when Unsafe was available. This had two consequences: 1. `DIRECT_MEMORY_COUNTER` was only initialized inside the `USE_DIRECT_BUFFER_NO_CLEANER=true` branch, so on any Java version without Unsafe the counter was always null even if the user explicitly set `io.netty.maxDirectMemory`. 2. The accounting calls themselves lived only in PlatformDependent's legacy NoCleaner methods (`allocateDirectNoCleaner`, `reallocateDirectNoCleaner`, `freeDirectNoCleaner`), which were only called by DirectCleaner. The other Cleaner implementations (`CleanerJava9`, `CleanerJava6`, `CleanerJava24Linker`, `CleanerJava25`) never called these methods and performed no accounting. The combined effect was that the configured limit was silently ignored on every path that didn't go through DirectCleaner: ``` Java version | Unsafe | Cleaner | Counter init | Accounting ───────────────|────────|─────────────────────|──────────────|─────────── 6-8 | Yes | DirectCleaner | Yes | Yes ✓ 9-23 | Yes | DirectCleaner | Yes | Yes ✓ 24 | Yes | DirectCleaner | Yes | Yes ✓ 24 | No | CleanerJava24Linker | No | No ✗ 25+ | No | CleanerJava24Linker | No | No ✗ 25+ | No | CleanerJava25 | No | No ✗ ``` On Java 25+, where Unsafe is disabled by default, this means `io.netty.maxDirectMemory` has no effect at all. **Modifications**: - Decouple `DIRECT_MEMORY_COUNTER` initialization from Unsafe availability. The counter is now created based solely on the value of `io.netty.maxDirectMemory`, independent of `USE_DIRECT_BUFFER_NO_CLEANER`. - Move accounting into each Cleaner's `CleanableDirectBufferImpl` so that every allocation/deallocation pair tracks memory regardless of which Cleaner is active: - `DirectCleaner`: increment in `CleanableDirectBufferImpl(int capacity)` constructor, decrement in `clean()`. - `CleanerJava9`: increment in constructor, decrement in `clean()`. - `CleanerJava6`: increment in constructor, decrement in `clean()`. - `CleanerJava24Linker`: increment before `malloc()`, decrement in `clean()`, with rollback on allocation failure. - `CleanerJava25`: increment in `allocate()` before MethodHandle invoke, decrement in `clean()` via finally block. - Change `incrementMemoryCounter`/`decrementMemoryCounter` from private to package-private so Cleaner implementations (same package) can call them directly. - Add a default `reallocate(CleanableDirectBuffer, int)` method to the Cleaner interface with an allocate-copy-free fallback. DirectCleaner overrides this with in-place `Unsafe.reallocateMemory`, adjusting the counter by the delta. - Add `PlatformDependent.reallocateDirect()` as the unified public entry point for reallocation. - Remove the legacy NoCleaner API surface from PlatformDependent: `allocateDirectNoCleaner`, `allocateDirectBufferNoCleaner`, `reallocateDirectNoCleaner`, `reallocateDirectBufferNoCleaner`, and `freeDirectNoCleaner`. - Remove `USE_DIRECT_BUFFER_NO_CLEANER` and `DIRECT_CLEANER` fields. `CLEANER` is now the single entry point; `useDirectBufferNoCleaner()` returns whether `CLEANER` is an instance of DirectCleaner. - Update `UnpooledUnsafeNoCleanerDirectByteBuf` to use the new unified API: remove the `allocateDirectBuffer()` override (parent's impl now does the right thing via `PlatformDependent.allocateDirect()`), and delegate `reallocateDirect()` to `PlatformDependent.reallocateDirect()`. - Update `PlatformDependentTest.testAllocateWithCapacity0()` to use the new CleanableDirectBuffer-based API. **Result**: After this change, the accounting matrix becomes: ``` Java version | Unsafe | Cleaner | Counter init | Accounting ───────────────|────────|─────────────────────|──────────────|─────────── 6-8 | Yes | DirectCleaner | Yes | Yes ✓ 9-23 | Yes | DirectCleaner | Yes | Yes ✓ 24 | Yes | DirectCleaner | Yes | Yes ✓ 24 | No | CleanerJava24Linker | Yes | Yes ✓ 25+ | No | CleanerJava24Linker | Yes | Yes ✓ 25+ | No | CleanerJava25 | Yes | Yes ✓ ``` `io.netty.maxDirectMemory` is now enforced on all Java versions and all Cleaner implementations. The legacy raw-ByteBuffer NoCleaner API surface is eliminated, and each `CleanableDirectBuffer` is responsible for its own accounting. --------- Co-authored-by: Chris Vest <mr.chrisvest@gmail.com>
1 parent 893ea2e commit 7937553

11 files changed

Lines changed: 118 additions & 123 deletions

File tree

‎buffer/src/main/java/io/netty/buffer/UnpooledByteBufAllocator.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public UnpooledByteBufAllocator(boolean preferDirect, boolean disableLeakDetecto
6868
* @param disableLeakDetector {@code true} if the leak-detection should be disabled completely for this
6969
* allocator. This can be useful if the user just want to depend on the GC to handle
7070
* direct buffers when not explicit released.
71-
* @param tryNoCleaner {@code true} if we should try to use {@link PlatformDependent#allocateDirectNoCleaner(int)}
71+
* @param tryNoCleaner {@code true} if we should try to use {@link PlatformDependent#allocateDirect(int)}
7272
* to allocate direct memory.
7373
*/
7474
public UnpooledByteBufAllocator(boolean preferDirect, boolean disableLeakDetector, boolean tryNoCleaner) {
@@ -195,10 +195,11 @@ protected CleanableDirectBuffer allocateDirectBuffer(int capacity, boolean permi
195195
}
196196

197197
@Override
198-
CleanableDirectBuffer reallocateDirect(CleanableDirectBuffer oldBuffer, int initialCapacity) {
199-
int capacity = oldBuffer.buffer().capacity();
200-
CleanableDirectBuffer buffer = super.reallocateDirect(oldBuffer, initialCapacity);
201-
return new DecrementingCleanableDirectBuffer(alloc(), buffer, buffer.buffer().capacity() - capacity);
198+
CleanableDirectBuffer reallocateDirect(CleanableDirectBuffer oldBuffer, int newCapacity) {
199+
int oldCapacity = oldBuffer.buffer().capacity();
200+
CleanableDirectBuffer buffer = super.reallocateDirect(oldBuffer, newCapacity);
201+
return new DecrementingCleanableDirectBuffer(
202+
alloc(), buffer, buffer.buffer().capacity() - oldCapacity);
202203
}
203204
}
204205

‎buffer/src/main/java/io/netty/buffer/UnpooledUnsafeNoCleanerDirectByteBuf.java‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,13 @@ class UnpooledUnsafeNoCleanerDirectByteBuf extends UnpooledUnsafeDirectByteBuf {
2525
super(alloc, initialCapacity, maxCapacity);
2626
}
2727

28-
@Override
29-
protected CleanableDirectBuffer allocateDirectBuffer(int capacity) {
30-
return PlatformDependent.allocateDirectBufferNoCleaner(capacity);
31-
}
32-
33-
@Override
34-
protected CleanableDirectBuffer allocateDirectBuffer(int capacity, boolean ignorePermitExpensiveClean) {
35-
return PlatformDependent.allocateDirectBufferNoCleaner(capacity);
36-
}
37-
3828
@Override
3929
protected ByteBuffer allocateDirect(int initialCapacity) {
4030
throw new UnsupportedOperationException();
4131
}
4232

43-
CleanableDirectBuffer reallocateDirect(CleanableDirectBuffer oldBuffer, int initialCapacity) {
44-
return PlatformDependent.reallocateDirectBufferNoCleaner(oldBuffer, initialCapacity);
33+
CleanableDirectBuffer reallocateDirect(CleanableDirectBuffer oldBuffer, int newCapacity) {
34+
return PlatformDependent.reallocateDirect(oldBuffer, newCapacity);
4535
}
4636

4737
@Override

‎common/src/main/java/io/netty/util/internal/Cleaner.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,26 @@ interface Cleaner {
3030
*/
3131
CleanableDirectBuffer allocate(int capacity);
3232

33+
/**
34+
* Reallocate a direct buffer with a new capacity. The old buffer is consumed and
35+
* must not be used after this call.
36+
* <p>
37+
* The default implementation allocates a new buffer, copies the data, and frees the old one.
38+
* Implementations may override this to provide more efficient reallocation (e.g. via
39+
* {@code Unsafe.reallocateMemory}).
40+
*/
41+
default CleanableDirectBuffer reallocate(CleanableDirectBuffer old, int newCapacity) {
42+
CleanableDirectBuffer newBuf = allocate(newCapacity);
43+
ByteBuffer oldBB = old.buffer();
44+
ByteBuffer newBB = newBuf.buffer();
45+
int bytesToCopy = Math.min(oldBB.capacity(), newCapacity);
46+
oldBB.position(0).limit(bytesToCopy);
47+
newBB.position(0).limit(bytesToCopy);
48+
newBB.put(oldBB).clear();
49+
old.clean();
50+
return newBuf;
51+
}
52+
3353
/**
3454
* Free a direct {@link ByteBuffer} if possible
3555
*

‎common/src/main/java/io/netty/util/internal/CleanerJava24Linker.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,19 @@ private static final class CleanableDirectBufferImpl implements CleanableDirectB
236236
private final long memoryAddress;
237237

238238
private CleanableDirectBufferImpl(int capacity) {
239-
long addr = malloc(capacity);
239+
PlatformDependent.incrementMemoryCounter(capacity);
240+
long addr;
241+
try {
242+
addr = malloc(capacity);
243+
} catch (Throwable e) {
244+
PlatformDependent.decrementMemoryCounter(capacity);
245+
throw e;
246+
}
240247
try {
241248
memoryAddress = addr;
242249
buffer = (ByteBuffer) INVOKE_CREATE_BYTEBUFFER.invokeExact(addr, (long) capacity);
243250
} catch (Throwable throwable) {
251+
PlatformDependent.decrementMemoryCounter(capacity);
244252
Error error = new Error(throwable);
245253
try {
246254
free(addr);
@@ -258,7 +266,9 @@ public ByteBuffer buffer() {
258266

259267
@Override
260268
public void clean() {
269+
int capacity = buffer.capacity();
261270
free(memoryAddress);
271+
PlatformDependent.decrementMemoryCounter(capacity);
262272
}
263273

264274
@Override

‎common/src/main/java/io/netty/util/internal/CleanerJava25.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,14 @@ static boolean isSupported() {
169169
@SuppressWarnings("OverlyStrongTypeCast") // The cast is needed for 'invokeExact' semantics.
170170
@Override
171171
public CleanableDirectBuffer allocate(int capacity) {
172+
PlatformDependent.incrementMemoryCounter(capacity);
172173
try {
173174
return (CleanableDirectBufferImpl) INVOKE_ALLOCATOR.invokeExact(capacity);
174175
} catch (RuntimeException e) {
176+
PlatformDependent.decrementMemoryCounter(capacity);
175177
throw e; // Propagate the runtime exceptions that the Arena would normally throw.
176178
} catch (Throwable e) {
179+
PlatformDependent.decrementMemoryCounter(capacity);
177180
throw new IllegalStateException("Unexpected allocation exception", e);
178181
}
179182
}
@@ -209,12 +212,15 @@ public ByteBuffer buffer() {
209212

210213
@Override
211214
public void clean() {
215+
int capacity = buffer.capacity();
212216
try {
213217
closeable.close();
214218
} catch (RuntimeException e) {
215219
throw e; // Propagate the runtime exceptions that Arena would normally throw.
216220
} catch (Exception e) {
217221
throw new IllegalStateException("Unexpected close exception", e);
222+
} finally {
223+
PlatformDependent.decrementMemoryCounter(capacity);
218224
}
219225
}
220226

‎common/src/main/java/io/netty/util/internal/CleanerJava6.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ private static final class CleanableDirectBufferImpl implements CleanableDirectB
156156

157157
private CleanableDirectBufferImpl(ByteBuffer buffer) {
158158
this.buffer = buffer;
159+
PlatformDependent.incrementMemoryCounter(buffer.capacity());
159160
}
160161

161162
@Override
@@ -165,7 +166,9 @@ public ByteBuffer buffer() {
165166

166167
@Override
167168
public void clean() {
169+
int capacity = buffer.capacity();
168170
freeDirectBufferStatic(buffer);
171+
PlatformDependent.decrementMemoryCounter(capacity);
169172
}
170173
}
171174
}

‎common/src/main/java/io/netty/util/internal/CleanerJava9.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ private static final class CleanableDirectBufferImpl implements CleanableDirectB
133133

134134
private CleanableDirectBufferImpl(ByteBuffer buffer) {
135135
this.buffer = buffer;
136+
PlatformDependent.incrementMemoryCounter(buffer.capacity());
136137
}
137138

138139
@Override
@@ -142,7 +143,9 @@ public ByteBuffer buffer() {
142143

143144
@Override
144145
public void clean() {
146+
int capacity = buffer.capacity();
145147
freeDirectBufferStatic(buffer);
148+
PlatformDependent.decrementMemoryCounter(capacity);
146149
}
147150
}
148151
}

‎common/src/main/java/io/netty/util/internal/DirectCleaner.java‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,50 @@
2020
final class DirectCleaner implements Cleaner {
2121
@Override
2222
public CleanableDirectBuffer allocate(int capacity) {
23-
return new CleanableDirectBufferImpl(PlatformDependent.allocateDirectNoCleaner(capacity));
23+
return new CleanableDirectBufferImpl(capacity);
24+
}
25+
26+
@Override
27+
public CleanableDirectBuffer reallocate(CleanableDirectBuffer old, int newCapacity) {
28+
int oldCapacity = old.buffer().capacity();
29+
int delta = newCapacity - oldCapacity;
30+
PlatformDependent.incrementMemoryCounter(delta);
31+
try {
32+
ByteBuffer newBuffer = PlatformDependent0.reallocateDirectNoCleaner(
33+
old.buffer(), newCapacity);
34+
return new CleanableDirectBufferImpl(newBuffer);
35+
} catch (Throwable e) {
36+
PlatformDependent.decrementMemoryCounter(delta);
37+
throw e;
38+
}
2439
}
2540

2641
@Override
2742
public void freeDirectBuffer(ByteBuffer buffer) {
28-
PlatformDependent.freeDirectNoCleaner(buffer);
43+
PlatformDependent0.freeMemory(PlatformDependent0.directBufferAddress(buffer));
2944
}
3045

3146
@Override
3247
public boolean hasExpensiveClean() {
3348
return false;
3449
}
3550

36-
CleanableDirectBuffer reallocate(CleanableDirectBuffer buffer, int capacity) {
37-
ByteBuffer newByteBuffer = PlatformDependent.reallocateDirectNoCleaner(buffer.buffer(), capacity);
38-
return new CleanableDirectBufferImpl(newByteBuffer);
39-
}
40-
4151
private static final class CleanableDirectBufferImpl implements CleanableDirectBuffer {
4252
private final ByteBuffer buffer;
4353

44-
private CleanableDirectBufferImpl(ByteBuffer buffer) {
54+
// Used for normal allocation — allocates memory and increments counter
55+
CleanableDirectBufferImpl(int capacity) {
56+
PlatformDependent.incrementMemoryCounter(capacity);
57+
try {
58+
this.buffer = PlatformDependent0.allocateDirectNoCleaner(capacity);
59+
} catch (Throwable e) {
60+
PlatformDependent.decrementMemoryCounter(capacity);
61+
throw e;
62+
}
63+
}
64+
65+
// Used for reallocation — memory already allocated, counter already adjusted
66+
CleanableDirectBufferImpl(ByteBuffer buffer) {
4567
this.buffer = buffer;
4668
}
4769

@@ -52,7 +74,9 @@ public ByteBuffer buffer() {
5274

5375
@Override
5476
public void clean() {
55-
PlatformDependent.freeDirectNoCleaner(buffer);
77+
int capacity = buffer.capacity();
78+
PlatformDependent0.freeMemory(PlatformDependent0.directBufferAddress(buffer));
79+
PlatformDependent.decrementMemoryCounter(capacity);
5680
}
5781
}
5882
}

‎common/src/main/java/io/netty/util/internal/OutOfDirectMemoryError.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import java.nio.ByteBuffer;
1919

2020
/**
21-
* {@link OutOfMemoryError} that is throws if {@link PlatformDependent#allocateDirectNoCleaner(int)} can not allocate
21+
* {@link OutOfMemoryError} that is throws if {@link PlatformDependent#allocateDirect(int)} can not allocate
2222
* a new {@link ByteBuffer} due memory restrictions.
2323
*/
2424
public final class OutOfDirectMemoryError extends OutOfMemoryError {

0 commit comments

Comments
 (0)