Merge pull request #401 from fyrz/RocksJava-Sigsegv-MergeOperatorName

[RocksJava] Fixes in MergeOperatorByName

java/Makefile

@@ -56,6 +56,7 @@ JAVA_TESTS = org.rocksdb.test.BackupableDBOptionsTest\
 		org.rocksdb.test.ComparatorTest\
 		org.rocksdb.test.DBOptionsTest\
 		org.rocksdb.test.DirectComparatorTest\
+		org.rocksdb.test.EnvironmentTest\
 		org.rocksdb.test.FilterTest\
 		org.rocksdb.test.FlushTest\
 		org.rocksdb.test.InfoLogLevelTest\
@@ -70,6 +71,7 @@ JAVA_TESTS = org.rocksdb.test.BackupableDBOptionsTest\
 		org.rocksdb.test.RocksDBTest\
 		org.rocksdb.test.RocksEnvTest\
 		org.rocksdb.test.RocksIteratorTest\
+		org.rocksdb.test.SizeUnitTest\
 		org.rocksdb.test.SnapshotTest\
 		org.rocksdb.test.StatisticsCollectorTest\
 		org.rocksdb.test.WriteBatchHandlerTest\
@@ -129,7 +131,7 @@ resolve_test_deps:
 	test -s "$(JAVA_ASSERTJ_JAR)" || curl -k -L -o "$(JAVA_ASSERTJ_JAR)" http://central.maven.org/maven2/org/assertj/assertj-core/1.7.0/assertj-core-1.7.0.jar
 
 test: java resolve_test_deps
-	java -ea -Djava.library.path=.:../ -cp "$(JAVA_TESTCLASSPATH)" org.rocksdb.test.RocksJunitRunner $(JAVA_TESTS)
+	java -ea -Xcheck:jni -Djava.library.path=.:../ -cp "$(JAVA_TESTCLASSPATH)" org.rocksdb.test.RocksJunitRunner $(JAVA_TESTS)
 
 db_bench: java
 	javac org/rocksdb/benchmark/*.java

java/org/rocksdb/ColumnFamilyOptions.java

@@ -127,6 +127,11 @@ public class ColumnFamilyOptions extends RocksObject
 
   @Override
   public ColumnFamilyOptions setMergeOperatorName(String name) {
+    assert (isInitialized());
+    if (name == null) {
+      throw new IllegalArgumentException(
+          "Merge operator name must not be null.");
+    }
     setMergeOperatorName(nativeHandle_, name);
     return this;
   }

java/org/rocksdb/Options.java

@@ -165,6 +165,11 @@ public class Options extends RocksObject
 
   @Override
   public Options setMergeOperatorName(String name) {
+    assert (isInitialized());
+    if (name == null) {
+      throw new IllegalArgumentException(
+          "Merge operator name must not be null.");
+    }
     setMergeOperatorName(nativeHandle_, name);
     return this;
   }

java/org/rocksdb/test/MergeTest.java

@@ -8,6 +8,7 @@ package org.rocksdb.test;
 import java.util.List;
 import java.util.ArrayList;
 
+import org.junit.Assert;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -252,4 +253,50 @@ public class MergeTest {
       }
     }
   }
+
+  @Test
+  public void emptyStringInSetMergeOperatorByName() {
+    Options opt = null;
+    ColumnFamilyOptions cOpt = null;
+    try {
+      opt = new Options();
+      cOpt = new ColumnFamilyOptions();
+      opt.setMergeOperatorName("");
+      cOpt.setMergeOperatorName("");
+    } finally {
+      if (opt != null) {
+        opt.dispose();
+      }
+      if (cOpt != null) {
+        cOpt.dispose();
+      }
+    }
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void nullStringInSetMergeOperatorByNameOptions() {
+    Options opt = null;
+    try {
+      opt = new Options();
+      opt.setMergeOperatorName(null);
+    } finally {
+      if (opt != null) {
+        opt.dispose();
+      }
+    }
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void
+      nullStringInSetMergeOperatorByNameColumnFamilyOptions() {
+    ColumnFamilyOptions opt = null;
+    try {
+      opt = new ColumnFamilyOptions();
+      opt.setMergeOperatorName(null);
+    } finally {
+      if (opt != null) {
+        opt.dispose();
+      }
+    }
+  }
 }

java/rocksjni.pom

@@ -118,7 +118,7 @@
                 <artifactId>maven-surefire-plugin</artifactId>
                 <version>2.17</version>
                 <configuration>
-                    <argLine>${argLine}</argLine>
+                    <argLine>${argLine} -Xcheck:jni</argLine>
                 </configuration>
             </plugin>
             <plugin>

java/rocksjni/options.cc

@@ -146,10 +146,12 @@ void Java_org_rocksdb_Options_setComparatorHandle__JJ(
  * Signature: (JJjava/lang/String)V
  */
 void Java_org_rocksdb_Options_setMergeOperatorName(
-    JNIEnv* env, jobject jobj, jlong jhandle, jstring name) {
+    JNIEnv* env, jobject jobj, jlong jhandle, jstring jop_name) {
-  const char* op_name = env->GetStringUTFChars(name, 0);
+  auto options = reinterpret_cast<rocksdb::Options*>(jhandle);
-  reinterpret_cast<rocksdb::Options*>(jhandle)->merge_operator =
+  const char* op_name = env->GetStringUTFChars(jop_name, 0);
-    rocksdb::MergeOperators::CreateFromStringId(op_name);
+  options->merge_operator = rocksdb::MergeOperators::CreateFromStringId(
+        op_name);
+  env->ReleaseStringUTFChars(jop_name, op_name);
 }
 
 /*
@@ -1884,10 +1886,12 @@ void Java_org_rocksdb_ColumnFamilyOptions_setComparatorHandle__JI(
  * Signature: (JJjava/lang/String)V
  */
 void Java_org_rocksdb_ColumnFamilyOptions_setMergeOperatorName(
-    JNIEnv* env, jobject jobj, jlong jhandle, jstring name) {
+    JNIEnv* env, jobject jobj, jlong jhandle, jstring jop_name) {
-  const char* op_name = env->GetStringUTFChars(name, 0);
+  auto options = reinterpret_cast<rocksdb::ColumnFamilyOptions*>(jhandle);
-  reinterpret_cast<rocksdb::ColumnFamilyOptions*>(jhandle)->merge_operator =
+  const char* op_name = env->GetStringUTFChars(jop_name, 0);
-    rocksdb::MergeOperators::CreateFromStringId(op_name);
+  options->merge_operator = rocksdb::MergeOperators::CreateFromStringId(
+        op_name);
+  env->ReleaseStringUTFChars(jop_name, op_name);
 }
 
 /*