Skip to content

Commit ddca4bd

Browse files
LambdAuroraLambdAurora
committed
Fix comparator lost buffer ref from proxy in Dbi.
TODO: address failing dbiWithComparatorThreadSafetyByteArray test.
1 parent 154df33 commit ddca4bd

File tree

4 files changed

+89
-31
lines changed

4 files changed

+89
-31
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ public final class Dbi<T> {
8181
if (nativeCb) {
8282
this.ccb =
8383
(keyA, keyB) -> {
84-
final T compKeyA = proxy.allocate();
85-
final T compKeyB = proxy.allocate();
86-
proxy.out(compKeyA, keyA, keyA.address());
87-
proxy.out(compKeyB, keyB, keyB.address());
84+
T compKeyA = proxy.allocate();
85+
T compKeyB = proxy.allocate();
86+
compKeyA = proxy.out(compKeyA, keyA, keyA.address());
87+
compKeyB = proxy.out(compKeyB, keyB, keyB.address());
8888
final int result = this.comparator.compare(compKeyA, compKeyB);
8989
proxy.deallocate(compKeyA);
9090
proxy.deallocate(compKeyB);

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,12 @@ public static Object[] data() {
6767
final ComparatorRunner string = new StringRunner();
6868
final ComparatorRunner db = new DirectBufferRunner();
6969
final ComparatorRunner ba = new ByteArrayRunner();
70+
final ComparatorRunner baUnsigned = new UnsignedByteArrayRunner();
7071
final ComparatorRunner bb = new ByteBufferRunner();
7172
final ComparatorRunner netty = new NettyRunner();
7273
final ComparatorRunner gub = new GuavaUnsignedBytes();
7374
final ComparatorRunner gsb = new GuavaSignedBytes();
74-
return new Object[] {string, db, ba, bb, netty, gub, gsb};
75+
return new Object[] {string, db, ba, baUnsigned, bb, netty, gub, gsb};
7576
}
7677

7778
private static byte[] buffer(final int... bytes) {
@@ -140,6 +141,16 @@ public int compare(final byte[] o1, final byte[] o2) {
140141
}
141142
}
142143

144+
/** Tests {@link ByteArrayProxy} (unsigned). */
145+
private static final class UnsignedByteArrayRunner implements ComparatorRunner {
146+
147+
@Override
148+
public int compare(final byte[] o1, final byte[] o2) {
149+
final Comparator<byte[]> c = PROXY_BA.getUnsignedComparator();
150+
return c.compare(o1, o2);
151+
}
152+
}
153+
143154
/** Tests {@link ByteBufferProxy}. */
144155
private static final class ByteBufferRunner implements ComparatorRunner {
145156

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

Lines changed: 68 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,7 @@
4646
import static org.lmdbjava.KeyRange.atMost;
4747
import static org.lmdbjava.PutFlags.MDB_NODUPDATA;
4848
import static org.lmdbjava.PutFlags.MDB_NOOVERWRITE;
49-
import static org.lmdbjava.TestUtils.DB_1;
50-
import static org.lmdbjava.TestUtils.ba;
51-
import static org.lmdbjava.TestUtils.bb;
49+
import static org.lmdbjava.TestUtils.*;
5250

5351
import java.io.File;
5452
import java.io.IOException;
@@ -63,7 +61,7 @@
6361
import java.util.concurrent.Future;
6462
import java.util.concurrent.TimeoutException;
6563
import java.util.concurrent.atomic.AtomicBoolean;
66-
import java.util.function.BiConsumer;
64+
import java.util.function.*;
6765
import org.agrona.concurrent.UnsafeBuffer;
6866
import org.hamcrest.Matchers;
6967
import org.junit.After;
@@ -82,6 +80,7 @@ public final class DbiTest {
8280

8381
@Rule public final TemporaryFolder tmp = new TemporaryFolder();
8482
private Env<ByteBuffer> env;
83+
private Env<byte[]> envBa;
8584

8685
@After
8786
public void after() {
@@ -97,6 +96,13 @@ public void before() throws IOException {
9796
.setMaxReaders(2)
9897
.setMaxDbs(2)
9998
.open(path, MDB_NOSUBDIR);
99+
final File pathBa = tmp.newFile();
100+
envBa =
101+
create(PROXY_BA)
102+
.setMapSize(MEBIBYTES.toBytes(64))
103+
.setMaxReaders(2)
104+
.setMaxDbs(2)
105+
.open(pathBa, MDB_NOSUBDIR);
100106
}
101107

102108
@Test(expected = ConstantDerivedException.class)
@@ -117,20 +123,41 @@ public void customComparator() {
117123
}
118124
return lexical * -1;
119125
};
120-
final Dbi<ByteBuffer> db = env.openDbi(DB_1, reverseOrder, true, MDB_CREATE);
121-
try (Txn<ByteBuffer> txn = env.txnWrite()) {
122-
assertThat(db.put(txn, bb(2), bb(3)), is(true));
123-
assertThat(db.put(txn, bb(4), bb(6)), is(true));
124-
assertThat(db.put(txn, bb(6), bb(7)), is(true));
125-
assertThat(db.put(txn, bb(8), bb(7)), is(true));
126+
doCustomComparator(env, reverseOrder, TestUtils::bb, ByteBuffer::getInt);
127+
}
128+
129+
@Test
130+
public void customComparatorByteArray() {
131+
final Comparator<byte[]> reverseOrder =
132+
(o1, o2) -> {
133+
final int lexical = PROXY_BA.getComparator().compare(o1, o2);
134+
if (lexical == 0) {
135+
return 0;
136+
}
137+
return lexical * -1;
138+
};
139+
doCustomComparator(envBa, reverseOrder, TestUtils::ba, TestUtils::fromBa);
140+
}
141+
142+
private <T> void doCustomComparator(
143+
Env<T> env,
144+
Comparator<T> comparator,
145+
IntFunction<T> serializer,
146+
ToIntFunction<T> deserializer) {
147+
final Dbi<T> db = env.openDbi(DB_1, comparator, true, MDB_CREATE);
148+
try (Txn<T> txn = env.txnWrite()) {
149+
assertThat(db.put(txn, serializer.apply(2), serializer.apply(3)), is(true));
150+
assertThat(db.put(txn, serializer.apply(4), serializer.apply(6)), is(true));
151+
assertThat(db.put(txn, serializer.apply(6), serializer.apply(7)), is(true));
152+
assertThat(db.put(txn, serializer.apply(8), serializer.apply(7)), is(true));
126153
txn.commit();
127154
}
128-
try (Txn<ByteBuffer> txn = env.txnRead();
129-
CursorIterable<ByteBuffer> ci = db.iterate(txn, atMost(bb(4)))) {
130-
final Iterator<KeyVal<ByteBuffer>> iter = ci.iterator();
131-
assertThat(iter.next().key().getInt(), is(8));
132-
assertThat(iter.next().key().getInt(), is(6));
133-
assertThat(iter.next().key().getInt(), is(4));
155+
try (Txn<T> txn = env.txnRead();
156+
CursorIterable<T> ci = db.iterate(txn, atMost(serializer.apply(4)))) {
157+
final Iterator<KeyVal<T>> iter = ci.iterator();
158+
assertThat(deserializer.applyAsInt(iter.next().key()), is(8));
159+
assertThat(deserializer.applyAsInt(iter.next().key()), is(6));
160+
assertThat(deserializer.applyAsInt(iter.next().key()), is(4));
134161
}
135162
}
136163

@@ -143,9 +170,24 @@ public void dbOpenMaxDatabases() {
143170

144171
@Test
145172
public void dbiWithComparatorThreadSafety() {
173+
doDbiWithComparatorThreadSafety(
174+
env, PROXY_OPTIMAL::getComparator, TestUtils::bb, ByteBuffer::getInt);
175+
}
176+
177+
@Test
178+
public void dbiWithComparatorThreadSafetyByteArray() {
179+
doDbiWithComparatorThreadSafety(
180+
envBa, PROXY_BA::getComparator, TestUtils::ba, TestUtils::fromBa);
181+
}
182+
183+
public <T> void doDbiWithComparatorThreadSafety(
184+
Env<T> env,
185+
Function<DbiFlags[], Comparator<T>> comparator,
186+
IntFunction<T> serializer,
187+
ToIntFunction<T> deserializer) {
146188
final DbiFlags[] flags = new DbiFlags[] {MDB_CREATE, MDB_INTEGERKEY};
147-
final Comparator<ByteBuffer> c = PROXY_OPTIMAL.getComparator(flags);
148-
final Dbi<ByteBuffer> db = env.openDbi(DB_1, c, true, flags);
189+
final Comparator<T> c = comparator.apply(flags);
190+
final Dbi<T> db = env.openDbi(DB_1, c, true, flags);
149191

150192
final List<Integer> keys = range(0, 1_000).boxed().collect(toList());
151193

@@ -155,25 +197,25 @@ public void dbiWithComparatorThreadSafety() {
155197
pool.submit(
156198
() -> {
157199
while (proceed.get()) {
158-
try (Txn<ByteBuffer> txn = env.txnRead()) {
159-
db.get(txn, bb(50));
200+
try (Txn<T> txn = env.txnRead()) {
201+
db.get(txn, serializer.apply(50));
160202
}
161203
}
162204
});
163205

164206
for (final Integer key : keys) {
165-
try (Txn<ByteBuffer> txn = env.txnWrite()) {
166-
db.put(txn, bb(key), bb(3));
207+
try (Txn<T> txn = env.txnWrite()) {
208+
db.put(txn, serializer.apply(key), serializer.apply(3));
167209
txn.commit();
168210
}
169211
}
170212

171-
try (Txn<ByteBuffer> txn = env.txnRead();
172-
CursorIterable<ByteBuffer> ci = db.iterate(txn)) {
173-
final Iterator<KeyVal<ByteBuffer>> iter = ci.iterator();
213+
try (Txn<T> txn = env.txnRead();
214+
CursorIterable<T> ci = db.iterate(txn)) {
215+
final Iterator<KeyVal<T>> iter = ci.iterator();
174216
final List<Integer> result = new ArrayList<>();
175217
while (iter.hasNext()) {
176-
result.add(iter.next().key().getInt());
218+
result.add(deserializer.applyAsInt(iter.next().key()));
177219
}
178220

179221
assertThat(result, Matchers.contains(keys.toArray(new Integer[0])));

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ static byte[] ba(final int value) {
4141
return b.byteArray();
4242
}
4343

44+
static int fromBa(final byte[] ba) {
45+
final MutableDirectBuffer b = new UnsafeBuffer(ba);
46+
return b.getInt(0);
47+
}
48+
4449
static ByteBuffer bb(final int value) {
4550
final ByteBuffer bb = allocateDirect(BYTES);
4651
bb.putInt(value).flip();

0 commit comments

Comments
 (0)