Skip to content

Commit a73383e

Browse files
committed
Minor fix in rocksdb jni library, RocksDB now does not implement Closeable.
Summary: * [java] RocksDB now does not implement Closeable. * [java] RocksDB.close() is now synchronized. * [c++] Fix a bug in rocksjni.cc that does not release a java reference before throwing an exception. Test Plan: make jni make jtest Reviewers: haobo, sdong Reviewed By: haobo CC: leveldb Differential Revision: https://reviews.facebook.net/D17355
1 parent 8e81caf commit a73383e

File tree

4 files changed

+17
-16
lines changed

4 files changed

+17
-16
lines changed

java/RocksDBSample.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ public static void main(String[] args) {
2121
String db_path = args[0];
2222

2323
System.out.println("RocksDBSample");
24+
RocksDB db = null;
2425

2526
try {
26-
RocksDB db = RocksDB.open(db_path);
27+
db = RocksDB.open(db_path);
2728
db.put("hello".getBytes(), "world".getBytes());
2829
byte[] value = db.get("hello".getBytes());
2930
System.out.format("Get('hello') = %s\n",
@@ -67,13 +68,11 @@ public static void main(String[] args) {
6768
assert(len == RocksDB.NOT_FOUND);
6869
len = db.get(testKey, enoughArray);
6970
assert(len == testValue.length);
70-
try {
71-
db.close();
72-
} catch (IOException e) {
73-
System.err.println(e);
74-
}
7571
} catch (RocksDBException e) {
7672
System.err.println(e);
7773
}
74+
if (db != null) {
75+
db.close();
76+
}
7877
}
7978
}

java/org/rocksdb/RocksDB.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* All methods of this class could potentially throw RocksDBException, which
1717
* indicates sth wrong at the rocksdb library side and the call failed.
1818
*/
19-
public class RocksDB implements Closeable {
19+
public class RocksDB {
2020
public static final int NOT_FOUND = -1;
2121
/**
2222
* The factory constructor of RocksDB that opens a RocksDB instance given
@@ -33,8 +33,8 @@ public static RocksDB open(String path) throws RocksDBException {
3333
return db;
3434
}
3535

36-
@Override public void close() throws IOException {
37-
if (nativeHandle != 0) {
36+
public synchronized void close() {
37+
if (nativeHandle_ != 0) {
3838
close0();
3939
}
4040
}
@@ -80,11 +80,15 @@ public byte[] get(byte[] key) throws RocksDBException {
8080
return get(key, key.length);
8181
}
8282

83+
@Override protected void finalize() {
84+
close();
85+
}
86+
8387
/**
8488
* Private constructor.
8589
*/
8690
private RocksDB() {
87-
nativeHandle = -1;
91+
nativeHandle_ = 0;
8892
}
8993

9094
// native methods
@@ -99,5 +103,5 @@ private native byte[] get(
99103
byte[] key, int keyLen) throws RocksDBException;
100104
private native void close0();
101105

102-
private long nativeHandle;
106+
private long nativeHandle_;
103107
}

java/rocksjni/portal.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class RocksDBJni {
2929
// that stores the pointer to rocksdb::DB.
3030
static jfieldID getHandleFieldID(JNIEnv* env) {
3131
static jfieldID fid = env->GetFieldID(
32-
getJClass(env), "nativeHandle", "J");
32+
getJClass(env), "nativeHandle_", "J");
3333
assert(fid != nullptr);
3434
return fid;
3535
}

java/rocksjni/rocksjni.cc

+2-4
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ jint Java_org_rocksdb_RocksDB_get___3BI_3BI(
147147
env->ReleaseByteArrayElements(jvalue, value, JNI_ABORT);
148148
return kNotFound;
149149
} else if (!s.ok()) {
150+
env->ReleaseByteArrayElements(jvalue, value, JNI_ABORT);
150151
// Here since we are throwing a Java exception from c++ side.
151152
// As a result, c++ does not know calling this function will in fact
152153
// throwing an exception. As a result, the execution flow will
@@ -164,10 +165,7 @@ jint Java_org_rocksdb_RocksDB_get___3BI_3BI(
164165

165166
memcpy(value, cvalue.c_str(), length);
166167
env->ReleaseByteArrayElements(jvalue, value, JNI_COMMIT);
167-
if (cvalue_len > length) {
168-
return static_cast<jint>(cvalue_len);
169-
}
170-
return length;
168+
return static_cast<jint>(cvalue_len);
171169
}
172170

173171
/*

0 commit comments

Comments
 (0)