Skip to content

Commit 30a93b4

Browse files
benalexaubenalexau
committed
Detect illegal multi-thread Txn use (see lmdbjava#23)
1 parent f45dbab commit 30a93b4

File tree

4 files changed

+127
-50
lines changed

4 files changed

+127
-50
lines changed

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

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import jnr.ffi.Pointer;
2828
import jnr.ffi.byref.PointerByReference;
2929
import static org.lmdbjava.ByteBufferProxy.PROXY_OPTIMAL;
30+
import static org.lmdbjava.EnvFlags.MDB_NOTLS;
3031
import static org.lmdbjava.EnvFlags.MDB_RDONLY_ENV;
3132
import static org.lmdbjava.Library.LIB;
3233
import org.lmdbjava.Library.MDB_envinfo;
@@ -59,17 +60,21 @@ public final class Env<T> implements AutoCloseable {
5960

6061
private boolean closed;
6162
private final int maxKeySize;
63+
private final boolean noTls;
6264
private final BufferProxy<T> proxy;
6365
private final Pointer ptr;
6466
private final boolean readOnly;
67+
private final ThreadLocal<Txn<T>> txns;
6568

6669
private Env(final BufferProxy<T> proxy, final Pointer ptr,
67-
final boolean readOnly) {
70+
final boolean noTls, final boolean readOnly) {
6871
this.proxy = proxy;
72+
this.noTls = noTls;
6973
this.readOnly = readOnly;
7074
this.ptr = ptr;
7175
// cache max key size to avoid further JNI calls
7276
this.maxKeySize = LIB.mdb_env_get_maxkeysize(ptr);
77+
this.txns = new ThreadLocal<>();
7378
}
7479

7580
/**
@@ -260,6 +265,21 @@ public void sync(final boolean force) {
260265
checkRc(LIB.mdb_env_sync(ptr, f));
261266
}
262267

268+
/**
269+
* Obtain the transaction associated with the current thread, if any.
270+
*
271+
* <p>
272+
* The transaction will only be returned until {@link Txn#close()}.
273+
*
274+
* <p>
275+
* Only a parent transaction will be returned.
276+
*
277+
* @return the transaction or null if this thread has no transaction
278+
*/
279+
public Txn<T> txn() {
280+
return txns.get();
281+
}
282+
263283
/**
264284
* Obtain a transaction with the requested parent and flags.
265285
*
@@ -271,7 +291,15 @@ public Txn<T> txn(final Txn<T> parent, final TxnFlags... flags) {
271291
if (closed) {
272292
throw new AlreadyClosedException();
273293
}
274-
return new Txn<>(this, parent, proxy, flags);
294+
final boolean limitTxnsPerThread = !readOnly || !noTls;
295+
if (limitTxnsPerThread && parent == null && txns.get() != null) {
296+
throw new ThreadHasTransactionException();
297+
}
298+
final Txn<T> tx = new Txn<>(this, parent, proxy, flags);
299+
if (parent == null) {
300+
txns.set(tx);
301+
}
302+
return tx;
275303
}
276304

277305
/**
@@ -280,10 +308,7 @@ public Txn<T> txn(final Txn<T> parent, final TxnFlags... flags) {
280308
* @return a read-only transaction
281309
*/
282310
public Txn<T> txnRead() {
283-
if (closed) {
284-
throw new AlreadyClosedException();
285-
}
286-
return new Txn<>(this, null, proxy, MDB_RDONLY_TXN);
311+
return txn(null, MDB_RDONLY_TXN);
287312
}
288313

289314
/**
@@ -292,16 +317,24 @@ public Txn<T> txnRead() {
292317
* @return a read-write transaction
293318
*/
294319
public Txn<T> txnWrite() {
295-
if (closed) {
296-
throw new AlreadyClosedException();
297-
}
298-
return new Txn<>(this, null, proxy);
320+
return txn(null);
299321
}
300322

301323
Pointer pointer() {
302324
return ptr;
303325
}
304326

327+
/**
328+
* Release the current transaction from thread local storage.
329+
*
330+
* @param txn to release
331+
*/
332+
void txnRelease(final Txn<T> txn) {
333+
if (txn.getParent() == null) {
334+
txns.remove();
335+
}
336+
}
337+
305338
/**
306339
* Object has already been closed and the operation is therefore prohibited.
307340
*/
@@ -373,9 +406,10 @@ public Env<T> open(final File path, final int mode,
373406
checkRc(LIB.mdb_env_set_maxdbs(ptr, maxDbs));
374407
checkRc(LIB.mdb_env_set_maxreaders(ptr, maxReaders));
375408
final int flagsMask = mask(flags);
409+
final boolean noTls = isSet(flagsMask, MDB_NOTLS);
376410
final boolean readOnly = isSet(flagsMask, MDB_RDONLY_ENV);
377411
checkRc(LIB.mdb_env_open(ptr, path.getAbsolutePath(), flagsMask, mode));
378-
return new Env<>(proxy, ptr, readOnly); // NOPMD
412+
return new Env<>(proxy, ptr, noTls, readOnly); // NOPMD
379413
} catch (final LmdbNativeException e) {
380414
LIB.mdb_env_close(ptr);
381415
throw e;
@@ -495,6 +529,21 @@ public static final class ReadersFullException extends LmdbNativeException {
495529
}
496530
}
497531

532+
/**
533+
* The current thread already has an unreleased (unclosed) transaction.
534+
*/
535+
public static class ThreadHasTransactionException extends LmdbException {
536+
537+
private static final long serialVersionUID = 1L;
538+
539+
/**
540+
* Creates a new instance.
541+
*/
542+
public ThreadHasTransactionException() {
543+
super("Current thread already has an unreleased (unclosed) transaction");
544+
}
545+
}
546+
498547
/**
499548
* Environment version mismatch.
500549
*/

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

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@
4444
* @param <T> buffer type
4545
*/
4646
public final class Txn<T> implements AutoCloseable {
47-
47+
4848
private static final MemoryManager MEM_MGR = RUNTIME.getMemoryManager();
49+
private final Env<T> env;
4950
private final T k;
5051
private final Txn<T> parent;
5152
private final BufferProxy<T> proxy;
@@ -57,9 +58,10 @@ public final class Txn<T> implements AutoCloseable {
5758
private final boolean readOnly;
5859
private State state;
5960
private final T v;
60-
61+
6162
Txn(final Env<T> env, final Txn<T> parent, final BufferProxy<T> proxy,
6263
final TxnFlags... flags) {
64+
this.env = env;
6365
this.proxy = proxy;
6466
final int flagsMask = mask(flags);
6567
this.readOnly = isSet(flagsMask, MDB_RDONLY_TXN);
@@ -74,14 +76,14 @@ public final class Txn<T> implements AutoCloseable {
7476
final Pointer txnParentPtr = parent == null ? null : parent.ptr;
7577
checkRc(LIB.mdb_txn_begin(env.pointer(), txnParentPtr, flagsMask, txnPtr));
7678
ptr = txnPtr.getPointer(0);
77-
79+
7880
this.k = proxy.allocate();
7981
this.v = proxy.allocate();
8082
ptrKey = MEM_MGR.allocateTemporary(MDB_VAL_STRUCT_SIZE, false);
8183
ptrKeyAddr = ptrKey.address();
8284
ptrVal = MEM_MGR.allocateTemporary(MDB_VAL_STRUCT_SIZE, false);
8385
ptrValAddr = ptrVal.address();
84-
86+
8587
state = READY;
8688
}
8789

@@ -113,6 +115,7 @@ public void close() {
113115
proxy.deallocate(k);
114116
proxy.deallocate(v);
115117
state = RELEASED;
118+
env.txnRelease(this);
116119
}
117120

118121
/**
@@ -199,19 +202,19 @@ public void reset() {
199202
public T val() {
200203
return v;
201204
}
202-
205+
203206
void checkReadOnly() throws ReadOnlyRequiredException {
204207
if (!readOnly) {
205208
throw new ReadOnlyRequiredException();
206209
}
207210
}
208-
211+
209212
void checkReady() throws NotReadyException {
210213
if (state != READY) {
211214
throw new NotReadyException();
212215
}
213216
}
214-
217+
215218
void checkWritesAllowed() throws ReadWriteRequiredException {
216219
if (readOnly) {
217220
throw new ReadWriteRequiredException();
@@ -226,7 +229,7 @@ void checkWritesAllowed() throws ReadWriteRequiredException {
226229
State getState() {
227230
return state;
228231
}
229-
232+
230233
void keyIn(final T key) {
231234
proxy.in(key, ptrKey, ptrKeyAddr);
232235
if (SHOULD_CHECK) {
@@ -236,23 +239,23 @@ void keyIn(final T key) {
236239
}
237240
}
238241
}
239-
242+
240243
void keyOut() {
241244
proxy.out(k, ptrKey, ptrKeyAddr);
242245
}
243-
246+
244247
Pointer pointer() {
245248
return ptr;
246249
}
247-
250+
248251
Pointer pointerKey() {
249252
return ptrKey;
250253
}
251-
254+
252255
Pointer pointerVal() {
253256
return ptrVal;
254257
}
255-
258+
256259
void valIn(final T val, final boolean zeroLenValAllowed) {
257260
proxy.in(val, ptrVal, ptrValAddr);
258261
if (SHOULD_CHECK && !zeroLenValAllowed) {
@@ -262,14 +265,14 @@ void valIn(final T val, final boolean zeroLenValAllowed) {
262265
}
263266
}
264267
}
265-
268+
266269
void valIn(final int size) {
267270
if (size <= 0) {
268271
throw new IndexOutOfBoundsException("Reservation must be > 0 bytes");
269272
}
270273
proxy.in(v, size, ptrVal, ptrValAddr);
271274
}
272-
275+
273276
void valOut() {
274277
proxy.out(v, ptrVal, ptrValAddr);
275278
}
@@ -278,10 +281,10 @@ void valOut() {
278281
* Transaction must abort, has a child, or is invalid.
279282
*/
280283
public static final class BadException extends LmdbNativeException {
281-
284+
282285
static final int MDB_BAD_TXN = -30_782;
283286
private static final long serialVersionUID = 1L;
284-
287+
285288
BadException() {
286289
super(MDB_BAD_TXN, "Transaction must abort, has a child, or is invalid");
287290
}
@@ -291,10 +294,10 @@ public static final class BadException extends LmdbNativeException {
291294
* Invalid reuse of reader locktable slot.
292295
*/
293296
public static final class BadReaderLockException extends LmdbNativeException {
294-
297+
295298
static final int MDB_BAD_RSLOT = -30_783;
296299
private static final long serialVersionUID = 1L;
297-
300+
298301
BadReaderLockException() {
299302
super(MDB_BAD_RSLOT, "Invalid reuse of reader locktable slot");
300303
}
@@ -304,7 +307,7 @@ public static final class BadReaderLockException extends LmdbNativeException {
304307
* The proposed R-W transaction is incompatible with a R-O Env.
305308
*/
306309
public static class EnvIsReadOnly extends LmdbException {
307-
310+
308311
private static final long serialVersionUID = 1L;
309312

310313
/**
@@ -319,7 +322,7 @@ public EnvIsReadOnly() {
319322
* The proposed transaction is incompatible with its parent transaction.
320323
*/
321324
public static class IncompatibleParent extends LmdbException {
322-
325+
323326
private static final long serialVersionUID = 1L;
324327

325328
/**
@@ -334,7 +337,7 @@ public IncompatibleParent() {
334337
* Transaction is not in a READY state.
335338
*/
336339
public static final class NotReadyException extends LmdbException {
337-
340+
338341
private static final long serialVersionUID = 1L;
339342

340343
/**
@@ -349,7 +352,7 @@ public NotReadyException() {
349352
* The current transaction has not been reset.
350353
*/
351354
public static class NotResetException extends LmdbException {
352-
355+
353356
private static final long serialVersionUID = 1L;
354357

355358
/**
@@ -364,7 +367,7 @@ public NotResetException() {
364367
* The current transaction is not a read-only transaction.
365368
*/
366369
public static class ReadOnlyRequiredException extends LmdbException {
367-
370+
368371
private static final long serialVersionUID = 1L;
369372

370373
/**
@@ -379,7 +382,7 @@ public ReadOnlyRequiredException() {
379382
* The current transaction is not a read-write transaction.
380383
*/
381384
public static class ReadWriteRequiredException extends LmdbException {
382-
385+
383386
private static final long serialVersionUID = 1L;
384387

385388
/**
@@ -394,7 +397,7 @@ public ReadWriteRequiredException() {
394397
* The current transaction has already been reset.
395398
*/
396399
public static class ResetException extends LmdbException {
397-
400+
398401
private static final long serialVersionUID = 1L;
399402

400403
/**
@@ -409,10 +412,10 @@ public ResetException() {
409412
* Transaction has too many dirty pages.
410413
*/
411414
public static final class TxFullException extends LmdbNativeException {
412-
415+
413416
static final int MDB_TXN_FULL = -30_788;
414417
private static final long serialVersionUID = 1L;
415-
418+
416419
TxFullException() {
417420
super(MDB_TXN_FULL, "Transaction has too many dirty pages");
418421
}
@@ -424,5 +427,5 @@ public static final class TxFullException extends LmdbNativeException {
424427
enum State {
425428
READY, DONE, RESET, RELEASED
426429
}
427-
430+
428431
}

0 commit comments

Comments
 (0)