Skip to content

Commit 3db0def

Browse files
benalexaubenalexau
committed
Add protection for closed env (#186)
1 parent 66d9f7a commit 3db0def

File tree

9 files changed

+349
-4
lines changed

9 files changed

+349
-4
lines changed

‎pom.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,14 @@
143143
</execution>
144144
</executions>
145145
</plugin>
146+
<plugin>
147+
<groupId>org.apache.maven.plugins</groupId>
148+
<artifactId>maven-surefire-plugin</artifactId>
149+
<configuration>
150+
<forkCount>1</forkCount>
151+
<reuseForks>false</reuseForks>
152+
</configuration>
153+
</plugin>
146154
<plugin>
147155
<groupId>org.basepom.maven</groupId>
148156
<artifactId>duplicate-finder-maven-plugin</artifactId>

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,15 @@ public final class Cursor<T> implements AutoCloseable {
5151
private final KeyVal<T> kv;
5252
private final Pointer ptrCursor;
5353
private Txn<T> txn;
54+
private final Env<T> env;
5455

55-
Cursor(final Pointer ptr, final Txn<T> txn) {
56+
Cursor(final Pointer ptr, final Txn<T> txn, final Env<T> env) {
5657
requireNonNull(ptr);
5758
requireNonNull(txn);
5859
this.ptrCursor = ptr;
5960
this.txn = txn;
6061
this.kv = txn.newKeyVal();
62+
this.env = env;
6163
}
6264

6365
/**
@@ -73,8 +75,11 @@ public void close() {
7375
return;
7476
}
7577
kv.close();
76-
if (SHOULD_CHECK && !txn.isReadOnly()) {
77-
txn.checkReady();
78+
if (SHOULD_CHECK) {
79+
env.checkNotClosed();
80+
if (!txn.isReadOnly()) {
81+
txn.checkReady();
82+
}
7883
}
7984
LIB.mdb_cursor_close(ptrCursor);
8085
closed = true;
@@ -91,6 +96,7 @@ public void close() {
9196
*/
9297
public long count() {
9398
if (SHOULD_CHECK) {
99+
env.checkNotClosed();
94100
checkNotClosed();
95101
txn.checkReady();
96102
}
@@ -109,6 +115,7 @@ public long count() {
109115
*/
110116
public void delete(final PutFlags... f) {
111117
if (SHOULD_CHECK) {
118+
env.checkNotClosed();
112119
checkNotClosed();
113120
txn.checkReady();
114121
txn.checkWritesAllowed();
@@ -138,6 +145,7 @@ public boolean get(final T key, final T data, final SeekOp op) {
138145
if (SHOULD_CHECK) {
139146
requireNonNull(key);
140147
requireNonNull(op);
148+
env.checkNotClosed();
141149
checkNotClosed();
142150
txn.checkReady();
143151
}
@@ -168,6 +176,7 @@ public boolean get(final T key, final GetOp op) {
168176
if (SHOULD_CHECK) {
169177
requireNonNull(key);
170178
requireNonNull(op);
179+
env.checkNotClosed();
171180
checkNotClosed();
172181
txn.checkReady();
173182
}
@@ -238,6 +247,7 @@ public boolean put(final T key, final T val, final PutFlags... op) {
238247
if (SHOULD_CHECK) {
239248
requireNonNull(key);
240249
requireNonNull(val);
250+
env.checkNotClosed();
241251
checkNotClosed();
242252
txn.checkReady();
243253
txn.checkWritesAllowed();
@@ -281,6 +291,7 @@ public void putMultiple(final T key, final T val, final int elements,
281291
requireNonNull(txn);
282292
requireNonNull(key);
283293
requireNonNull(val);
294+
env.checkNotClosed();
284295
txn.checkReady();
285296
txn.checkWritesAllowed();
286297
}
@@ -311,6 +322,7 @@ public void putMultiple(final T key, final T val, final int elements,
311322
public void renew(final Txn<T> newTxn) {
312323
if (SHOULD_CHECK) {
313324
requireNonNull(newTxn);
325+
env.checkNotClosed();
314326
checkNotClosed();
315327
this.txn.checkReadOnly(); // existing
316328
newTxn.checkReadOnly();
@@ -339,6 +351,7 @@ public void renew(final Txn<T> newTxn) {
339351
public T reserve(final T key, final int size, final PutFlags... op) {
340352
if (SHOULD_CHECK) {
341353
requireNonNull(key);
354+
env.checkNotClosed();
342355
checkNotClosed();
343356
txn.checkReady();
344357
txn.checkWritesAllowed();
@@ -361,6 +374,7 @@ public T reserve(final T key, final int size, final PutFlags... op) {
361374
public boolean seek(final SeekOp op) {
362375
if (SHOULD_CHECK) {
363376
requireNonNull(op);
377+
env.checkNotClosed();
364378
checkNotClosed();
365379
txn.checkReady();
366380
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ public final class Dbi<T> {
104104
*/
105105
public void close() {
106106
clean();
107+
if (SHOULD_CHECK) {
108+
env.checkNotClosed();
109+
}
107110
LIB.mdb_dbi_close(env.pointer(), ptr);
108111
}
109112

@@ -199,6 +202,7 @@ public void drop(final Txn<T> txn) {
199202
public void drop(final Txn<T> txn, final boolean delete) {
200203
if (SHOULD_CHECK) {
201204
requireNonNull(txn);
205+
env.checkNotClosed();
202206
txn.checkReady();
203207
txn.checkWritesAllowed();
204208
}
@@ -228,6 +232,7 @@ public T get(final Txn<T> txn, final T key) {
228232
if (SHOULD_CHECK) {
229233
requireNonNull(txn);
230234
requireNonNull(key);
235+
env.checkNotClosed();
231236
txn.checkReady();
232237
}
233238
txn.kv().keyIn(key);
@@ -295,6 +300,7 @@ public CursorIterable<T> iterate(final Txn<T> txn, final KeyRange<T> range,
295300
if (SHOULD_CHECK) {
296301
requireNonNull(txn);
297302
requireNonNull(range);
303+
env.checkNotClosed();
298304
txn.checkReady();
299305
}
300306
final Comparator<T> useComp;
@@ -313,6 +319,9 @@ public CursorIterable<T> iterate(final Txn<T> txn, final KeyRange<T> range,
313319
* @return the list of flags this Dbi was created with
314320
*/
315321
public List<DbiFlags> listFlags(final Txn<T> txn) {
322+
if (SHOULD_CHECK) {
323+
env.checkNotClosed();
324+
}
316325
final IntByReference resultPtr = new IntByReference();
317326
checkRc(LIB.mdb_dbi_flags(txn.pointer(), ptr, resultPtr));
318327

@@ -348,11 +357,12 @@ public List<DbiFlags> listFlags(final Txn<T> txn) {
348357
public Cursor<T> openCursor(final Txn<T> txn) {
349358
if (SHOULD_CHECK) {
350359
requireNonNull(txn);
360+
env.checkNotClosed();
351361
txn.checkReady();
352362
}
353363
final PointerByReference cursorPtr = new PointerByReference();
354364
checkRc(LIB.mdb_cursor_open(txn.pointer(), ptr, cursorPtr));
355-
return new Cursor<>(cursorPtr.getValue(), txn);
365+
return new Cursor<>(cursorPtr.getValue(), txn, env);
356366
}
357367

358368
/**
@@ -392,6 +402,7 @@ public boolean put(final Txn<T> txn, final T key, final T val,
392402
requireNonNull(txn);
393403
requireNonNull(key);
394404
requireNonNull(val);
405+
env.checkNotClosed();
395406
txn.checkReady();
396407
txn.checkWritesAllowed();
397408
}
@@ -434,6 +445,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,
434445
if (SHOULD_CHECK) {
435446
requireNonNull(txn);
436447
requireNonNull(key);
448+
env.checkNotClosed();
437449
txn.checkReady();
438450
txn.checkWritesAllowed();
439451
}
@@ -455,6 +467,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,
455467
public Stat stat(final Txn<T> txn) {
456468
if (SHOULD_CHECK) {
457469
requireNonNull(txn);
470+
env.checkNotClosed();
458471
txn.checkReady();
459472
}
460473
final MDB_stat stat = new MDB_stat(RUNTIME);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,12 @@ Pointer pointer() {
423423
return ptr;
424424
}
425425

426+
void checkNotClosed() {
427+
if (closed) {
428+
throw new AlreadyClosedException();
429+
}
430+
}
431+
426432
private void validateDirectoryEmpty(final File path) {
427433
if (!path.exists()) {
428434
throw new InvalidCopyDestination("Path does not exist");

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import static jnr.ffi.Memory.allocateDirect;
2424
import static jnr.ffi.NativeType.ADDRESS;
25+
import static org.lmdbjava.Env.SHOULD_CHECK;
2526
import static org.lmdbjava.Library.LIB;
2627
import static org.lmdbjava.Library.RUNTIME;
2728
import static org.lmdbjava.MaskedFlag.isSet;
@@ -49,6 +50,7 @@ public final class Txn<T> implements AutoCloseable {
4950
private final BufferProxy<T> proxy;
5051
private final Pointer ptr;
5152
private final boolean readOnly;
53+
private final Env<T> env;
5254
private State state;
5355

5456
Txn(final Env<T> env, final Txn<T> parent, final BufferProxy<T> proxy,
@@ -60,6 +62,7 @@ public final class Txn<T> implements AutoCloseable {
6062
if (env.isReadOnly() && !this.readOnly) {
6163
throw new EnvIsReadOnly();
6264
}
65+
this.env = env;
6366
this.parent = parent;
6467
if (parent != null && parent.isReadOnly() != this.readOnly) {
6568
throw new IncompatibleParent();
@@ -76,6 +79,9 @@ public final class Txn<T> implements AutoCloseable {
7679
* Aborts this transaction.
7780
*/
7881
public void abort() {
82+
if (SHOULD_CHECK) {
83+
env.checkNotClosed();
84+
}
7985
checkReady();
8086
state = DONE;
8187
LIB.mdb_txn_abort(ptr);
@@ -91,6 +97,9 @@ public void abort() {
9197
*/
9298
@Override
9399
public void close() {
100+
if (SHOULD_CHECK) {
101+
env.checkNotClosed();
102+
}
94103
if (state == RELEASED) {
95104
return;
96105
}
@@ -105,6 +114,9 @@ public void close() {
105114
* Commits this transaction.
106115
*/
107116
public void commit() {
117+
if (SHOULD_CHECK) {
118+
env.checkNotClosed();
119+
}
108120
checkReady();
109121
state = DONE;
110122
checkRc(LIB.mdb_txn_commit(ptr));
@@ -116,6 +128,9 @@ public void commit() {
116128
* @return A transaction ID, valid if input is an active transaction
117129
*/
118130
public long getId() {
131+
if (SHOULD_CHECK) {
132+
env.checkNotClosed();
133+
}
119134
return LIB.mdb_txn_id(ptr);
120135
}
121136

@@ -153,6 +168,9 @@ public T key() {
153168
* Renews a read-only transaction previously released by {@link #reset()}.
154169
*/
155170
public void renew() {
171+
if (SHOULD_CHECK) {
172+
env.checkNotClosed();
173+
}
156174
if (state != RESET) {
157175
throw new NotResetException();
158176
}
@@ -166,6 +184,9 @@ public void renew() {
166184
* can be reused upon calling {@link #renew()}.
167185
*/
168186
public void reset() {
187+
if (SHOULD_CHECK) {
188+
env.checkNotClosed();
189+
}
169190
checkReadOnly();
170191
if (state != READY && state != DONE) {
171192
throw new ResetException();

‎src/test/java/org/lmdbjava/CursorIterableTest.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import java.util.NoSuchElementException;
6464

6565
import com.google.common.primitives.UnsignedBytes;
66+
import org.hamcrest.Matchers;
6667
import org.junit.After;
6768
import org.junit.Before;
6869
import org.junit.Rule;
@@ -302,6 +303,59 @@ public void removeOddElements() {
302303
verify(all(), 4, 8);
303304
}
304305

306+
@Test(expected = Env.AlreadyClosedException.class)
307+
public void nextWithClosedEnvTest() {
308+
try (Txn<ByteBuffer> txn = env.txnRead()) {
309+
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
310+
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();
311+
312+
env.close();
313+
c.next();
314+
}
315+
}
316+
}
317+
318+
@Test(expected = Env.AlreadyClosedException.class)
319+
public void removeWithClosedEnvTest() {
320+
try (Txn<ByteBuffer> txn = env.txnWrite()) {
321+
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
322+
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();
323+
324+
final KeyVal<ByteBuffer> keyVal = c.next();
325+
assertThat(keyVal, Matchers.notNullValue());
326+
327+
env.close();
328+
c.remove();
329+
}
330+
}
331+
}
332+
333+
@Test(expected = Env.AlreadyClosedException.class)
334+
public void hasNextWithClosedEnvTest() {
335+
try (Txn<ByteBuffer> txn = env.txnRead()) {
336+
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
337+
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();
338+
339+
env.close();
340+
c.hasNext();
341+
}
342+
}
343+
}
344+
345+
@Test(expected = Env.AlreadyClosedException.class)
346+
public void forEachRemainingWithClosedEnvTest() {
347+
try (Txn<ByteBuffer> txn = env.txnRead()) {
348+
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
349+
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();
350+
351+
env.close();
352+
c.forEachRemaining(keyVal -> {
353+
354+
});
355+
}
356+
}
357+
}
358+
305359
private void verify(final KeyRange<ByteBuffer> range, final int... expected) {
306360
verify(range, null, expected);
307361
}

0 commit comments

Comments
 (0)