[RocksJava] - Hardening RocksIterator
RocksIterator will sometimes Sigsegv on dispose. Mainly thats related to dispose order. If the related RocksDB instance is freed beforehand RocksIterator.dispose() will fail. Within this commit there is a major change to RocksIterator. RocksIterator will hold a private reference to the RocksDB instance which created the RocksIterator. So even if RocksDB is freed in the same GC cycle the RocksIterator instances will be freed prior to related RocksDB instances. Another aspect targets the dispose logic if the RocksDB is freed previously and already gc`ed. On dispose of a RocksIterator the dispose logic will check if the RocksDB instance points to an initialized DB. If not the dispose logic will not perform any further action. The crash can be reproduced by using the related test provided within this commit. Related information: This relates to @adamretter`s facebook rocksdb-dev group post about SigSegv on RocksIterator.dispose().
This commit is contained in:
parent
c1c68bce43
commit
56ef2caaa5
@ -46,6 +46,7 @@ test: java
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.OptionsTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.ReadOnlyTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.ReadOptionsTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.RocksIteratorTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.SnapshotTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.StatisticsCollectorTest
|
||||
java -ea -Djava.library.path=.:../ -cp "$(ROCKSDB_JAR):.:./*" org.rocksdb.test.ComparatorOptionsTest
|
||||
|
@ -888,7 +888,7 @@ public class RocksDB extends RocksObject {
|
||||
* @return instance of iterator object.
|
||||
*/
|
||||
public RocksIterator newIterator() {
|
||||
return new RocksIterator(iterator0(nativeHandle_));
|
||||
return new RocksIterator(this, iterator0(nativeHandle_));
|
||||
}
|
||||
|
||||
|
||||
@ -936,7 +936,8 @@ public class RocksDB extends RocksObject {
|
||||
* @return instance of iterator object.
|
||||
*/
|
||||
public RocksIterator newIterator(ColumnFamilyHandle columnFamilyHandle) {
|
||||
return new RocksIterator(iterator0(nativeHandle_, columnFamilyHandle.nativeHandle_));
|
||||
return new RocksIterator(this, iterator0(nativeHandle_,
|
||||
columnFamilyHandle.nativeHandle_));
|
||||
}
|
||||
|
||||
/**
|
||||
@ -958,7 +959,7 @@ public class RocksDB extends RocksObject {
|
||||
|
||||
long[] iteratorRefs = iterators(nativeHandle_, columnFamilyHandleList);
|
||||
for (int i=0; i<columnFamilyHandleList.size(); i++){
|
||||
iterators.add(new RocksIterator(iteratorRefs[i]));
|
||||
iterators.add(new RocksIterator(this, iteratorRefs[i]));
|
||||
}
|
||||
return iterators;
|
||||
}
|
||||
|
@ -19,9 +19,10 @@ package org.rocksdb;
|
||||
* @see org.rocksdb.RocksObject
|
||||
*/
|
||||
public class RocksIterator extends RocksObject {
|
||||
public RocksIterator(long nativeHandle) {
|
||||
public RocksIterator(RocksDB rocksDB, long nativeHandle) {
|
||||
super();
|
||||
nativeHandle_ = nativeHandle;
|
||||
rocksDB_ = rocksDB;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -129,8 +130,10 @@ public class RocksIterator extends RocksObject {
|
||||
*/
|
||||
@Override protected void disposeInternal() {
|
||||
assert(isInitialized());
|
||||
if (rocksDB_.isInitialized()) {
|
||||
disposeInternal(nativeHandle_);
|
||||
}
|
||||
}
|
||||
|
||||
private native boolean isValid0(long handle);
|
||||
private native void disposeInternal(long handle);
|
||||
@ -142,4 +145,6 @@ public class RocksIterator extends RocksObject {
|
||||
private native byte[] value0(long handle);
|
||||
private native void seek0(long handle, byte[] target, int targetLen);
|
||||
private native void status0(long handle);
|
||||
|
||||
RocksDB rocksDB_;
|
||||
}
|
||||
|
48
java/org/rocksdb/test/RocksIteratorTest.java
Normal file
48
java/org/rocksdb/test/RocksIteratorTest.java
Normal file
@ -0,0 +1,48 @@
|
||||
// Copyright (c) 2014, Facebook, Inc. All rights reserved.
|
||||
// This source code is licensed under the BSD-style license found in the
|
||||
// LICENSE file in the root directory of this source tree. An additional grant
|
||||
// of patent rights can be found in the PATENTS file in the same directory.
|
||||
package org.rocksdb.test;
|
||||
|
||||
import org.rocksdb.ColumnFamilyHandle;
|
||||
import org.rocksdb.Options;
|
||||
import org.rocksdb.RocksDB;
|
||||
import org.rocksdb.RocksDBException;
|
||||
import org.rocksdb.RocksIterator;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
public class RocksIteratorTest {
|
||||
static final String DB_PATH = "/tmp/rocksdbjni_iterator_test";
|
||||
static {
|
||||
RocksDB.loadLibrary();
|
||||
}
|
||||
|
||||
public static void main(String[] args){
|
||||
RocksDB db;
|
||||
Options options = new Options();
|
||||
options.setCreateIfMissing(true)
|
||||
.setCreateMissingColumnFamilies(true);
|
||||
try {
|
||||
db = RocksDB.open(options, DB_PATH);
|
||||
db.put("key".getBytes(), "value".getBytes());
|
||||
RocksIterator iter = db.newIterator();
|
||||
RocksIterator iter2 = db.newIterator();
|
||||
RocksIterator iter3 = db.newIterator();
|
||||
iter = null;
|
||||
db.close();
|
||||
db = null;
|
||||
iter2 = null;
|
||||
System.gc();
|
||||
System.runFinalization();
|
||||
System.out.println("Passed RocksIterator Test");
|
||||
iter3.dispose();
|
||||
System.gc();
|
||||
System.runFinalization();
|
||||
}catch (RocksDBException e){
|
||||
e.printStackTrace();
|
||||
assert(false);
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user