Skip to content

Commit c91893a

Browse files
benalexaubenalexau
committed
Fix transient pointer deallocation in ByteArrayProxy
JNR-FFI's TransientNativeMemory was prematurely deallocating off-heap memory before LMDB syscalls completed. Return transient pointers from BufferProxy.in() and use reachability fencing at all call sites to prevent premature GC. BufferProxy and KeyVal were also simplified to remove pointer address passing, given this is inexpensively available from the Pointer.address() accessor (which is backed by a final field). This is an API breaking change if external users implemented their own BufferProxy, however it is considered unlikely many (if any) users would have ever done this. Fixes #252
1 parent 1ea5902 commit c91893a

File tree

9 files changed

+91
-57
lines changed

9 files changed

+91
-57
lines changed

‎src/main/java/org/lmdbjava/BufferProxy.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,30 +107,29 @@ protected Comparator<T> getComparator(DbiFlags... flags) {
107107
*
108108
* @param buffer the buffer to write to <code>MDB_val</code>
109109
* @param ptr the pointer to the <code>MDB_val</code>
110-
* @param ptrAddr the address of the <code>MDB_val</code> pointer
110+
* @return a transient pointer that must be kept alive, or null if none
111111
*/
112-
protected abstract void in(T buffer, Pointer ptr, long ptrAddr);
112+
protected abstract Pointer in(T buffer, Pointer ptr);
113113

114114
/**
115115
* Called when the <code>MDB_val</code> should be set to reflect the passed buffer.
116116
*
117117
* @param buffer the buffer to write to <code>MDB_val</code>
118118
* @param size the buffer size to write to <code>MDB_val</code>
119119
* @param ptr the pointer to the <code>MDB_val</code>
120-
* @param ptrAddr the address of the <code>MDB_val</code> pointer
120+
* @return a transient pointer that must be kept alive, or null if none
121121
*/
122-
protected abstract void in(T buffer, int size, Pointer ptr, long ptrAddr);
122+
protected abstract Pointer in(T buffer, int size, Pointer ptr);
123123

124124
/**
125125
* Called when the <code>MDB_val</code> may have changed and the passed buffer should be modified
126126
* to reflect the new <code>MDB_val</code>.
127127
*
128128
* @param buffer the buffer to write to <code>MDB_val</code>
129129
* @param ptr the pointer to the <code>MDB_val</code>
130-
* @param ptrAddr the address of the <code>MDB_val</code> pointer
131130
* @return the buffer for <code>MDB_val</code>
132131
*/
133-
protected abstract T out(T buffer, Pointer ptr, long ptrAddr);
132+
protected abstract T out(T buffer, Pointer ptr);
134133

135134
/**
136135
* Create a new {@link KeyVal} to hold pointers for this buffer proxy.

‎src/main/java/org/lmdbjava/ByteArrayProxy.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,20 +114,22 @@ protected Comparator<byte[]> getUnsignedComparator() {
114114
}
115115

116116
@Override
117-
protected void in(final byte[] buffer, final Pointer ptr, final long ptrAddr) {
117+
protected Pointer in(final byte[] buffer, final Pointer ptr) {
118118
final Pointer pointer = MEM_MGR.allocateDirect(buffer.length);
119119
pointer.put(0, buffer, 0, buffer.length);
120120
ptr.putLong(STRUCT_FIELD_OFFSET_SIZE, buffer.length);
121121
ptr.putAddress(STRUCT_FIELD_OFFSET_DATA, pointer.address());
122+
return pointer;
122123
}
123124

124125
@Override
125-
protected void in(final byte[] buffer, final int size, final Pointer ptr, final long ptrAddr) {
126+
protected Pointer in(final byte[] buffer, final int size, final Pointer ptr) {
126127
// cannot reserve for byte arrays
128+
return null;
127129
}
128130

129131
@Override
130-
protected byte[] out(final byte[] buffer, final Pointer ptr, final long ptrAddr) {
132+
protected byte[] out(final byte[] buffer, final Pointer ptr) {
131133
final long addr = ptr.getAddress(STRUCT_FIELD_OFFSET_DATA);
132134
final int size = (int) ptr.getLong(STRUCT_FIELD_OFFSET_SIZE);
133135
final Pointer pointer = MEM_MGR.newPointer(addr, size);

‎src/main/java/org/lmdbjava/ByteBufProxy.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,21 +136,26 @@ protected byte[] getBytes(final ByteBuf buffer) {
136136
}
137137

138138
@Override
139-
protected void in(final ByteBuf buffer, final Pointer ptr, final long ptrAddr) {
139+
protected Pointer in(final ByteBuf buffer, final Pointer ptr) {
140+
final long ptrAddr = ptr.address();
140141
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, buffer.writerIndex() - buffer.readerIndex());
141142
UNSAFE.putLong(
142143
ptrAddr + STRUCT_FIELD_OFFSET_DATA, buffer.memoryAddress() + buffer.readerIndex());
144+
return null;
143145
}
144146

145147
@Override
146-
protected void in(final ByteBuf buffer, final int size, final Pointer ptr, final long ptrAddr) {
148+
protected Pointer in(final ByteBuf buffer, final int size, final Pointer ptr) {
149+
final long ptrAddr = ptr.address();
147150
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, size);
148151
UNSAFE.putLong(
149152
ptrAddr + STRUCT_FIELD_OFFSET_DATA, buffer.memoryAddress() + buffer.readerIndex());
153+
return null;
150154
}
151155

152156
@Override
153-
protected ByteBuf out(final ByteBuf buffer, final Pointer ptr, final long ptrAddr) {
157+
protected ByteBuf out(final ByteBuf buffer, final Pointer ptr) {
158+
final long ptrAddr = ptr.address();
154159
final long addr = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA);
155160
final long size = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE);
156161
UNSAFE.putLong(buffer, addressOffset, addr);

‎src/main/java/org/lmdbjava/ByteBufferProxy.java‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -221,20 +221,22 @@ private static final class ReflectiveProxy extends AbstractByteBufferProxy {
221221
}
222222

223223
@Override
224-
protected void in(final ByteBuffer buffer, final Pointer ptr, final long ptrAddr) {
224+
protected Pointer in(final ByteBuffer buffer, final Pointer ptr) {
225225
ptr.putAddress(STRUCT_FIELD_OFFSET_DATA, address(buffer));
226226
ptr.putLong(STRUCT_FIELD_OFFSET_SIZE, buffer.remaining());
227+
return null;
227228
}
228229

229230
@Override
230-
protected void in(
231-
final ByteBuffer buffer, final int size, final Pointer ptr, final long ptrAddr) {
231+
protected Pointer in(final ByteBuffer buffer, final int size, final Pointer ptr) {
232232
ptr.putLong(STRUCT_FIELD_OFFSET_SIZE, size);
233233
ptr.putAddress(STRUCT_FIELD_OFFSET_DATA, address(buffer));
234+
return null;
234235
}
235236

236237
@Override
237-
protected ByteBuffer out(final ByteBuffer buffer, final Pointer ptr, final long ptrAddr) {
238+
protected ByteBuffer out(final ByteBuffer buffer, final Pointer ptr) {
239+
final long ptrAddr = ptr.address();
238240
final long addr = ptr.getAddress(STRUCT_FIELD_OFFSET_DATA);
239241
final long size = ptr.getLong(STRUCT_FIELD_OFFSET_SIZE);
240242
try {
@@ -269,20 +271,24 @@ private static final class UnsafeProxy extends AbstractByteBufferProxy {
269271
}
270272

271273
@Override
272-
protected void in(final ByteBuffer buffer, final Pointer ptr, final long ptrAddr) {
274+
protected Pointer in(final ByteBuffer buffer, final Pointer ptr) {
275+
final long ptrAddr = ptr.address();
273276
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, buffer.remaining());
274277
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA, address(buffer));
278+
return null;
275279
}
276280

277281
@Override
278-
protected void in(
279-
final ByteBuffer buffer, final int size, final Pointer ptr, final long ptrAddr) {
282+
protected Pointer in(final ByteBuffer buffer, final int size, final Pointer ptr) {
283+
final long ptrAddr = ptr.address();
280284
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, size);
281285
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA, address(buffer));
286+
return null;
282287
}
283288

284289
@Override
285-
protected ByteBuffer out(final ByteBuffer buffer, final Pointer ptr, final long ptrAddr) {
290+
protected ByteBuffer out(final ByteBuffer buffer, final Pointer ptr) {
291+
final long ptrAddr = ptr.address();
286292
final long addr = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA);
287293
final long size = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE);
288294
UNSAFE.putLong(buffer, ADDRESS_OFFSET, addr);

‎src/main/java/org/lmdbjava/Cursor.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ public boolean get(final T key, final T data, final SeekOp op) {
141141
checkNotClosed();
142142
txn.checkReady();
143143
}
144-
kv.keyIn(key);
145-
kv.valIn(data);
144+
final Pointer transientKey = kv.keyIn(key);
145+
final Pointer transientVal = kv.valIn(data);
146146

147147
final int rc = LIB.mdb_cursor_get(ptrCursor, kv.pointerKey(), kv.pointerVal(), op.getCode());
148148

@@ -153,6 +153,10 @@ public boolean get(final T key, final T data, final SeekOp op) {
153153
checkRc(rc);
154154
kv.keyOut();
155155
kv.valOut();
156+
ReferenceUtil.reachabilityFence0(transientKey);
157+
ReferenceUtil.reachabilityFence0(transientVal);
158+
ReferenceUtil.reachabilityFence0(kv.key());
159+
ReferenceUtil.reachabilityFence0(kv.val());
156160
ReferenceUtil.reachabilityFence0(key);
157161
return true;
158162
}
@@ -172,7 +176,7 @@ public boolean get(final T key, final GetOp op) {
172176
checkNotClosed();
173177
txn.checkReady();
174178
}
175-
kv.keyIn(key);
179+
final Pointer transientKey = kv.keyIn(key);
176180

177181
final int rc = LIB.mdb_cursor_get(ptrCursor, kv.pointerKey(), kv.pointerVal(), op.getCode());
178182

@@ -183,6 +187,9 @@ public boolean get(final T key, final GetOp op) {
183187
checkRc(rc);
184188
kv.keyOut();
185189
kv.valOut();
190+
ReferenceUtil.reachabilityFence0(transientKey);
191+
ReferenceUtil.reachabilityFence0(kv.key());
192+
ReferenceUtil.reachabilityFence0(kv.val());
186193
ReferenceUtil.reachabilityFence0(key);
187194
return true;
188195
}
@@ -243,8 +250,8 @@ public boolean put(final T key, final T val, final PutFlags... op) {
243250
txn.checkReady();
244251
txn.checkWritesAllowed();
245252
}
246-
kv.keyIn(key);
247-
kv.valIn(val);
253+
final Pointer transientKey = kv.keyIn(key);
254+
final Pointer transientVal = kv.valIn(val);
248255
final int mask = mask(true, op);
249256
final int rc = LIB.mdb_cursor_put(ptrCursor, kv.pointerKey(), kv.pointerVal(), mask);
250257
if (rc == MDB_KEYEXIST) {
@@ -256,6 +263,8 @@ public boolean put(final T key, final T val, final PutFlags... op) {
256263
return false;
257264
}
258265
checkRc(rc);
266+
ReferenceUtil.reachabilityFence0(transientKey);
267+
ReferenceUtil.reachabilityFence0(transientVal);
259268
ReferenceUtil.reachabilityFence0(key);
260269
ReferenceUtil.reachabilityFence0(val);
261270
return true;
@@ -287,10 +296,12 @@ public void putMultiple(final T key, final T val, final int elements, final PutF
287296
if (SHOULD_CHECK && !isSet(mask, MDB_MULTIPLE)) {
288297
throw new IllegalArgumentException("Must set " + MDB_MULTIPLE + " flag");
289298
}
290-
txn.kv().keyIn(key);
299+
final Pointer transientKey = txn.kv().keyIn(key);
291300
final Pointer dataPtr = txn.kv().valInMulti(val, elements);
292301
final int rc = LIB.mdb_cursor_put(ptrCursor, txn.kv().pointerKey(), dataPtr, mask);
293302
checkRc(rc);
303+
ReferenceUtil.reachabilityFence0(transientKey);
304+
ReferenceUtil.reachabilityFence0(dataPtr);
294305
ReferenceUtil.reachabilityFence0(key);
295306
ReferenceUtil.reachabilityFence0(val);
296307
}
@@ -340,11 +351,13 @@ public T reserve(final T key, final int size, final PutFlags... op) {
340351
txn.checkReady();
341352
txn.checkWritesAllowed();
342353
}
343-
kv.keyIn(key);
344-
kv.valIn(size);
354+
final Pointer transientKey = kv.keyIn(key);
355+
final Pointer transientVal = kv.valIn(size);
345356
final int flags = mask(true, op) | MDB_RESERVE.getMask();
346357
checkRc(LIB.mdb_cursor_put(ptrCursor, kv.pointerKey(), kv.pointerVal(), flags));
347358
kv.valOut();
359+
ReferenceUtil.reachabilityFence0(transientKey);
360+
ReferenceUtil.reachabilityFence0(transientVal);
348361
ReferenceUtil.reachabilityFence0(key);
349362
return val();
350363
}

‎src/main/java/org/lmdbjava/Dbi.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ public final class Dbi<T> {
8383
(keyA, keyB) -> {
8484
final T compKeyA = proxy.allocate();
8585
final T compKeyB = proxy.allocate();
86-
proxy.out(compKeyA, keyA, keyA.address());
87-
proxy.out(compKeyB, keyB, keyB.address());
86+
proxy.out(compKeyA, keyA);
87+
proxy.out(compKeyB, keyB);
8888
final int result = this.comparator.compare(compKeyA, compKeyB);
8989
proxy.deallocate(compKeyA);
9090
proxy.deallocate(compKeyB);
@@ -161,18 +161,21 @@ public boolean delete(final Txn<T> txn, final T key, final T val) {
161161
txn.checkWritesAllowed();
162162
}
163163

164-
txn.kv().keyIn(key);
164+
final Pointer transientKey = txn.kv().keyIn(key);
165165

166166
Pointer data = null;
167+
Pointer transientVal = null;
167168
if (val != null) {
168-
txn.kv().valIn(val);
169+
transientVal = txn.kv().valIn(val);
169170
data = txn.kv().pointerVal();
170171
}
171172
final int rc = LIB.mdb_del(txn.pointer(), ptr, txn.kv().pointerKey(), data);
172173
if (rc == MDB_NOTFOUND) {
173174
return false;
174175
}
175176
checkRc(rc);
177+
ReferenceUtil.reachabilityFence0(transientKey);
178+
ReferenceUtil.reachabilityFence0(transientVal);
176179
ReferenceUtil.reachabilityFence0(key);
177180
ReferenceUtil.reachabilityFence0(val);
178181
return true;
@@ -232,14 +235,16 @@ public T get(final Txn<T> txn, final T key) {
232235
env.checkNotClosed();
233236
txn.checkReady();
234237
}
235-
txn.kv().keyIn(key);
238+
final Pointer transientKey = txn.kv().keyIn(key);
236239
final int rc = LIB.mdb_get(txn.pointer(), ptr, txn.kv().pointerKey(), txn.kv().pointerVal());
237240
if (rc == MDB_NOTFOUND) {
238241
return null;
239242
}
240243
checkRc(rc);
244+
final T result = txn.kv().valOut(); // marked as out in LMDB C docs
245+
ReferenceUtil.reachabilityFence0(transientKey);
241246
ReferenceUtil.reachabilityFence0(key);
242-
return txn.kv().valOut(); // marked as out in LMDB C docs
247+
return result;
243248
}
244249

245250
/**
@@ -366,8 +371,8 @@ public boolean put(final Txn<T> txn, final T key, final T val, final PutFlags...
366371
txn.checkReady();
367372
txn.checkWritesAllowed();
368373
}
369-
txn.kv().keyIn(key);
370-
txn.kv().valIn(val);
374+
final Pointer transientKey = txn.kv().keyIn(key);
375+
final Pointer transientVal = txn.kv().valIn(val);
371376
final int mask = mask(true, flags);
372377
final int rc =
373378
LIB.mdb_put(txn.pointer(), ptr, txn.kv().pointerKey(), txn.kv().pointerVal(), mask);
@@ -380,6 +385,8 @@ public boolean put(final Txn<T> txn, final T key, final T val, final PutFlags...
380385
return false;
381386
}
382387
checkRc(rc);
388+
ReferenceUtil.reachabilityFence0(transientKey);
389+
ReferenceUtil.reachabilityFence0(transientVal);
383390
ReferenceUtil.reachabilityFence0(key);
384391
ReferenceUtil.reachabilityFence0(val);
385392
return true;
@@ -408,11 +415,13 @@ public T reserve(final Txn<T> txn, final T key, final int size, final PutFlags..
408415
txn.checkReady();
409416
txn.checkWritesAllowed();
410417
}
411-
txn.kv().keyIn(key);
412-
txn.kv().valIn(size);
418+
final Pointer transientKey = txn.kv().keyIn(key);
419+
final Pointer transientVal = txn.kv().valIn(size);
413420
final int flags = mask(true, op) | MDB_RESERVE.getMask();
414421
checkRc(LIB.mdb_put(txn.pointer(), ptr, txn.kv().pointerKey(), txn.kv().pointerVal(), flags));
415422
txn.kv().valOut(); // marked as in,out in LMDB C docs
423+
ReferenceUtil.reachabilityFence0(transientKey);
424+
ReferenceUtil.reachabilityFence0(transientVal);
416425
ReferenceUtil.reachabilityFence0(key);
417426
return txn.val();
418427
}

‎src/main/java/org/lmdbjava/DirectBufferProxy.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,23 +134,27 @@ protected byte[] getBytes(final DirectBuffer buffer) {
134134
}
135135

136136
@Override
137-
protected void in(final DirectBuffer buffer, final Pointer ptr, final long ptrAddr) {
137+
protected Pointer in(final DirectBuffer buffer, final Pointer ptr) {
138+
final long ptrAddr = ptr.address();
138139
final long addr = buffer.addressOffset();
139140
final long size = buffer.capacity();
140141
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA, addr);
141142
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, size);
143+
return null;
142144
}
143145

144146
@Override
145-
protected void in(
146-
final DirectBuffer buffer, final int size, final Pointer ptr, final long ptrAddr) {
147+
protected Pointer in(final DirectBuffer buffer, final int size, final Pointer ptr) {
148+
final long ptrAddr = ptr.address();
147149
final long addr = buffer.addressOffset();
148150
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA, addr);
149151
UNSAFE.putLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE, size);
152+
return null;
150153
}
151154

152155
@Override
153-
protected DirectBuffer out(final DirectBuffer buffer, final Pointer ptr, final long ptrAddr) {
156+
protected DirectBuffer out(final DirectBuffer buffer, final Pointer ptr) {
157+
final long ptrAddr = ptr.address();
154158
final long addr = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_DATA);
155159
final long size = UNSAFE.getLong(ptrAddr + STRUCT_FIELD_OFFSET_SIZE);
156160
buffer.wrap(addr, (int) size);

0 commit comments

Comments
 (0)