Skip to content

gh-185 Add protection for closed env #186

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

Closed
at055612 wants to merge 4 commits into from
Closed

gh-185 Add protection for closed env #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3da5b06
gh-185 Add protection for closed env
at055612 Nov 4, 2021
303c9c0
gh-185 Add env.checkNotClosed to Dbi.put
at055612 Nov 4, 2021
1bc841e
gh-185 Remove redundant test
at055612 Nov 4, 2021
46e41ff
gh-185 Remove unused imports
at055612 Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/main/java/org/lmdbjava/Cursor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ public final class Cursor<T> implements AutoCloseable {
private final KeyVal<T> kv;
private final Pointer ptrCursor;
private Txn<T> txn;
private final Env<T> env;

Cursor(final Pointer ptr, final Txn<T> txn) {
Cursor(final Pointer ptr, final Txn<T> txn, final Env<T> env) {
requireNonNull(ptr);
requireNonNull(txn);
this.ptrCursor = ptr;
this.txn = txn;
this.kv = txn.newKeyVal();
this.env = env;
}

/**
Expand All @@ -72,8 +74,11 @@ public void close() {
if (closed) {
return;
}
if (SHOULD_CHECK && !txn.isReadOnly()) {
txn.checkReady();
if (SHOULD_CHECK) {
env.checkNotClosed();
if (!txn.isReadOnly()) {
txn.checkReady();
}
}
LIB.mdb_cursor_close(ptrCursor);
kv.close();
Expand All @@ -91,6 +96,7 @@ public void close() {
*/
public long count() {
if (SHOULD_CHECK) {
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
}
Expand All @@ -109,6 +115,7 @@ public long count() {
*/
public void delete(final PutFlags... f) {
if (SHOULD_CHECK) {
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
Expand Down Expand Up @@ -138,6 +145,7 @@ public boolean get(final T key, final T data, final SeekOp op) {
if (SHOULD_CHECK) {
requireNonNull(key);
requireNonNull(op);
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
}
Expand Down Expand Up @@ -168,6 +176,7 @@ public boolean get(final T key, final GetOp op) {
if (SHOULD_CHECK) {
requireNonNull(key);
requireNonNull(op);
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
}
Expand Down Expand Up @@ -238,6 +247,7 @@ public boolean put(final T key, final T val, final PutFlags... op) {
if (SHOULD_CHECK) {
requireNonNull(key);
requireNonNull(val);
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
Expand Down Expand Up @@ -281,6 +291,7 @@ public void putMultiple(final T key, final T val, final int elements,
requireNonNull(txn);
requireNonNull(key);
requireNonNull(val);
env.checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
}
Expand Down Expand Up @@ -311,6 +322,7 @@ public void putMultiple(final T key, final T val, final int elements,
public void renew(final Txn<T> newTxn) {
if (SHOULD_CHECK) {
requireNonNull(newTxn);
env.checkNotClosed();
checkNotClosed();
this.txn.checkReadOnly(); // existing
newTxn.checkReadOnly();
Expand Down Expand Up @@ -339,6 +351,7 @@ public void renew(final Txn<T> newTxn) {
public T reserve(final T key, final int size, final PutFlags... op) {
if (SHOULD_CHECK) {
requireNonNull(key);
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
Expand All @@ -361,6 +374,7 @@ public T reserve(final T key, final int size, final PutFlags... op) {
public boolean seek(final SeekOp op) {
if (SHOULD_CHECK) {
requireNonNull(op);
env.checkNotClosed();
checkNotClosed();
txn.checkReady();
}
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/org/lmdbjava/Dbi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ public final class Dbi<T> {
*/
public void close() {
clean();
if (SHOULD_CHECK) {
env.checkNotClosed();
}
LIB.mdb_dbi_close(env.pointer(), ptr);
}

Expand Down Expand Up @@ -199,6 +202,7 @@ public void drop(final Txn<T> txn) {
public void drop(final Txn<T> txn, final boolean delete) {
if (SHOULD_CHECK) {
requireNonNull(txn);
env.checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
}
Expand Down Expand Up @@ -228,6 +232,7 @@ public T get(final Txn<T> txn, final T key) {
if (SHOULD_CHECK) {
requireNonNull(txn);
requireNonNull(key);
env.checkNotClosed();
txn.checkReady();
}
txn.kv().keyIn(key);
Expand Down Expand Up @@ -295,6 +300,7 @@ public CursorIterable<T> iterate(final Txn<T> txn, final KeyRange<T> range,
if (SHOULD_CHECK) {
requireNonNull(txn);
requireNonNull(range);
env.checkNotClosed();
txn.checkReady();
}
final Comparator<T> useComp;
Expand All @@ -313,6 +319,9 @@ public CursorIterable<T> iterate(final Txn<T> txn, final KeyRange<T> range,
* @return the list of flags this Dbi was created with
*/
public List<DbiFlags> listFlags(final Txn<T> txn) {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
final IntByReference resultPtr = new IntByReference();
checkRc(LIB.mdb_dbi_flags(txn.pointer(), ptr, resultPtr));

Expand Down Expand Up @@ -348,11 +357,12 @@ public List<DbiFlags> listFlags(final Txn<T> txn) {
public Cursor<T> openCursor(final Txn<T> txn) {
if (SHOULD_CHECK) {
requireNonNull(txn);
env.checkNotClosed();
txn.checkReady();
}
final PointerByReference cursorPtr = new PointerByReference();
checkRc(LIB.mdb_cursor_open(txn.pointer(), ptr, cursorPtr));
return new Cursor<>(cursorPtr.getValue(), txn);
return new Cursor<>(cursorPtr.getValue(), txn, env);
}

/**
Expand Down Expand Up @@ -392,6 +402,7 @@ public boolean put(final Txn<T> txn, final T key, final T val,
requireNonNull(txn);
requireNonNull(key);
requireNonNull(val);
env.checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
}
Expand Down Expand Up @@ -434,6 +445,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,
if (SHOULD_CHECK) {
requireNonNull(txn);
requireNonNull(key);
env.checkNotClosed();
txn.checkReady();
txn.checkWritesAllowed();
}
Expand All @@ -455,6 +467,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,
public Stat stat(final Txn<T> txn) {
if (SHOULD_CHECK) {
requireNonNull(txn);
env.checkNotClosed();
txn.checkReady();
}
final MDB_stat stat = new MDB_stat(RUNTIME);
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/lmdbjava/Env.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,12 @@ Pointer pointer() {
return ptr;
}

void checkNotClosed() {
if (closed) {
throw new AlreadyClosedException();
}
}

private void validateDirectoryEmpty(final File path) {
if (!path.exists()) {
throw new InvalidCopyDestination("Path does not exist");
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/org/lmdbjava/Txn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import static jnr.ffi.Memory.allocateDirect;
import static jnr.ffi.NativeType.ADDRESS;
import static org.lmdbjava.Env.SHOULD_CHECK;
import static org.lmdbjava.Library.LIB;
import static org.lmdbjava.Library.RUNTIME;
import static org.lmdbjava.MaskedFlag.isSet;
Expand Down Expand Up @@ -49,6 +50,7 @@ public final class Txn<T> implements AutoCloseable {
private final BufferProxy<T> proxy;
private final Pointer ptr;
private final boolean readOnly;
private final Env<T> env;
private State state;

Txn(final Env<T> env, final Txn<T> parent, final BufferProxy<T> proxy,
Expand All @@ -60,6 +62,7 @@ public final class Txn<T> implements AutoCloseable {
if (env.isReadOnly() && !this.readOnly) {
throw new EnvIsReadOnly();
}
this.env = env;
this.parent = parent;
if (parent != null && parent.isReadOnly() != this.readOnly) {
throw new IncompatibleParent();
Expand All @@ -76,6 +79,9 @@ public final class Txn<T> implements AutoCloseable {
* Aborts this transaction.
*/
public void abort() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
checkReady();
state = DONE;
LIB.mdb_txn_abort(ptr);
Expand All @@ -91,6 +97,9 @@ public void abort() {
*/
@Override
public void close() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
if (state == RELEASED) {
return;
}
Expand All @@ -105,6 +114,9 @@ public void close() {
* Commits this transaction.
*/
public void commit() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
checkReady();
state = DONE;
checkRc(LIB.mdb_txn_commit(ptr));
Expand All @@ -116,6 +128,9 @@ public void commit() {
* @return A transaction ID, valid if input is an active transaction
*/
public long getId() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
return LIB.mdb_txn_id(ptr);
}

Expand Down Expand Up @@ -153,6 +168,9 @@ public T key() {
* Renews a read-only transaction previously released by {@link #reset()}.
*/
public void renew() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
if (state != RESET) {
throw new NotResetException();
}
Expand All @@ -166,6 +184,9 @@ public void renew() {
* can be reused upon calling {@link #renew()}.
*/
public void reset() {
if (SHOULD_CHECK) {
env.checkNotClosed();
}
checkReadOnly();
if (state != READY && state != DONE) {
throw new ResetException();
Expand Down
54 changes: 54 additions & 0 deletions src/test/java/org/lmdbjava/CursorIterableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.util.NoSuchElementException;

import com.google.common.primitives.UnsignedBytes;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -302,6 +303,59 @@ public void removeOddElements() {
verify(all(), 4, 8);
}

@Test(expected = Env.AlreadyClosedException.class)
public void nextWithClosedEnvTest() {
try (Txn<ByteBuffer> txn = env.txnRead()) {
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();

env.close();
c.next();
}
}
}

@Test(expected = Env.AlreadyClosedException.class)
public void removeWithClosedEnvTest() {
try (Txn<ByteBuffer> txn = env.txnWrite()) {
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();

final KeyVal<ByteBuffer> keyVal = c.next();
assertThat(keyVal, Matchers.notNullValue());

env.close();
c.remove();
}
}
}

@Test(expected = Env.AlreadyClosedException.class)
public void hasNextWithClosedEnvTest() {
try (Txn<ByteBuffer> txn = env.txnRead()) {
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();

env.close();
c.hasNext();
}
}
}

@Test(expected = Env.AlreadyClosedException.class)
public void forEachRemainingWithClosedEnvTest() {
try (Txn<ByteBuffer> txn = env.txnRead()) {
try (CursorIterable<ByteBuffer> ci = db.iterate(txn, KeyRange.all())) {
final Iterator<KeyVal<ByteBuffer>> c = ci.iterator();

env.close();
c.forEachRemaining(keyVal -> {

});
}
}
}

private void verify(final KeyRange<ByteBuffer> range, final int... expected) {
verify(range, null, expected);
}
Expand Down
Loading