Repository: carbondata Updated Branches: refs/heads/master b8d602598 -> 0fa0a96c4
[CARBONDATA-3108][CARBONDATA-3044] Fix the error of jvm will crash when CarbonRow use wrong index number in CSDK 1. fix the error of jvm will crash when CarbonRow use wrong index number in CSDK including getString, getVarchar, getArray, getDecimal 2. delete/release the data after running 3. init the variable to NULL This closes #2929 Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/0fa0a96c Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/0fa0a96c Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/0fa0a96c Branch: refs/heads/master Commit: 0fa0a96c4d55e4bba9bdf118fa7c3e37cd4713a7 Parents: b8d6025 Author: xubo245 <xub...@huawei.com> Authored: Mon Nov 19 22:48:31 2018 +0800 Committer: kunal642 <kunalkapoor...@gmail.com> Committed: Wed Nov 21 23:04:07 2018 +0530 ---------------------------------------------------------------------- store/CSDK/src/CarbonReader.cpp | 7 +- store/CSDK/src/CarbonReader.h | 5 + store/CSDK/src/CarbonRow.cpp | 60 +++++++-- store/CSDK/src/CarbonRow.h | 6 + store/CSDK/src/CarbonWriter.cpp | 3 + store/CSDK/test/main.cpp | 124 +++++++++++++++---- .../org/apache/carbondata/sdk/file/RowUtil.java | 7 +- 7 files changed, 177 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/src/CarbonReader.cpp ---------------------------------------------------------------------- diff --git a/store/CSDK/src/CarbonReader.cpp b/store/CSDK/src/CarbonReader.cpp index 8375b73..8ee65d0 100644 --- a/store/CSDK/src/CarbonReader.cpp +++ b/store/CSDK/src/CarbonReader.cpp @@ -31,7 +31,7 @@ void CarbonReader::builder(JNIEnv *env, char *path, char *tableName) { throw std::runtime_error("tableName parameter can't be NULL."); } jniEnv = env; - jclass carbonReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonReader"); + carbonReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonReader"); if (carbonReaderClass == NULL) { throw std::runtime_error("Can't find the class in java: org/apache/carbondata/sdk/file/CarbonReader"); } @@ -56,7 +56,7 @@ void CarbonReader::builder(JNIEnv *env, char *path) { throw std::runtime_error("path parameter can't be NULL."); } jniEnv = env; - jclass carbonReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonReader"); + carbonReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonReader"); if (carbonReaderClass == NULL) { throw std::runtime_error("Can't find the class in java: org/apache/carbondata/sdk/file/CarbonReader"); } @@ -230,4 +230,7 @@ void CarbonReader::close() { if (jniEnv->ExceptionCheck()) { throw jniEnv->ExceptionOccurred(); } + jniEnv->DeleteLocalRef(carbonReaderBuilderObject); + jniEnv->DeleteLocalRef(carbonReaderObject); + jniEnv->DeleteLocalRef(carbonReaderClass); } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/src/CarbonReader.h ---------------------------------------------------------------------- diff --git a/store/CSDK/src/CarbonReader.h b/store/CSDK/src/CarbonReader.h index 246e24b..9a1daeb 100644 --- a/store/CSDK/src/CarbonReader.h +++ b/store/CSDK/src/CarbonReader.h @@ -46,6 +46,11 @@ private: jobject carbonReaderObject = NULL; /** + * carbonReader class for reading data + */ + jclass carbonReaderClass = NULL; + + /** * Return true if carbonReaderBuilder Object isn't NULL * Throw exception if carbonReaderBuilder Object is NULL * http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/src/CarbonRow.cpp ---------------------------------------------------------------------- diff --git a/store/CSDK/src/CarbonRow.cpp b/store/CSDK/src/CarbonRow.cpp index f7066ec..45cf8f6 100644 --- a/store/CSDK/src/CarbonRow.cpp +++ b/store/CSDK/src/CarbonRow.cpp @@ -87,6 +87,9 @@ CarbonRow::CarbonRow(JNIEnv *env) { if (getArrayId == NULL) { throw std::runtime_error("Can't find the method in java: getArray"); } + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } } void CarbonRow::setCarbonRow(jobject data) { @@ -114,7 +117,11 @@ short CarbonRow::getShort(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticShortMethodA(rowUtilClass, getShortId, args); + short result = jniEnv->CallStaticShortMethodA(rowUtilClass, getShortId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } int CarbonRow::getInt(int ordinal) { @@ -123,7 +130,11 @@ int CarbonRow::getInt(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticIntMethodA(rowUtilClass, getIntId, args); + int result = jniEnv->CallStaticIntMethodA(rowUtilClass, getIntId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } long CarbonRow::getLong(int ordinal) { @@ -132,7 +143,11 @@ long CarbonRow::getLong(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticLongMethodA(rowUtilClass, getLongId, args); + long result = jniEnv->CallStaticLongMethodA(rowUtilClass, getLongId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } double CarbonRow::getDouble(int ordinal) { @@ -141,17 +156,24 @@ double CarbonRow::getDouble(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticDoubleMethodA(rowUtilClass, getDoubleId, args); + double result = jniEnv->CallStaticDoubleMethodA(rowUtilClass, getDoubleId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } - float CarbonRow::getFloat(int ordinal) { checkCarbonRow(); checkOrdinal(ordinal); jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticFloatMethodA(rowUtilClass, getFloatId, args); + float result = jniEnv->CallStaticFloatMethodA(rowUtilClass, getFloatId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } jboolean CarbonRow::getBoolean(int ordinal) { @@ -160,7 +182,11 @@ jboolean CarbonRow::getBoolean(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return jniEnv->CallStaticBooleanMethodA(rowUtilClass, getBooleanId, args); + bool result = jniEnv->CallStaticBooleanMethodA(rowUtilClass, getBooleanId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; } char *CarbonRow::getString(int ordinal) { @@ -170,6 +196,9 @@ char *CarbonRow::getString(int ordinal) { args[0].l = carbonRow; args[1].i = ordinal; jobject data = jniEnv->CallStaticObjectMethodA(rowUtilClass, getStringId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } char *str = (char *) jniEnv->GetStringUTFChars((jstring) data, JNI_FALSE); jniEnv->DeleteLocalRef(data); return str; @@ -182,6 +211,9 @@ char *CarbonRow::getDecimal(int ordinal) { args[0].l = carbonRow; args[1].i = ordinal; jobject data = jniEnv->CallStaticObjectMethodA(rowUtilClass, getDecimalId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } char *str = (char *) jniEnv->GetStringUTFChars((jstring) data, JNI_FALSE); jniEnv->DeleteLocalRef(data); return str; @@ -194,6 +226,9 @@ char *CarbonRow::getVarchar(int ordinal) { args[0].l = carbonRow; args[1].i = ordinal; jobject data = jniEnv->CallStaticObjectMethodA(rowUtilClass, getVarcharId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } char *str = (char *) jniEnv->GetStringUTFChars((jstring) data, JNI_FALSE); jniEnv->DeleteLocalRef(data); return str; @@ -205,5 +240,14 @@ jobjectArray CarbonRow::getArray(int ordinal) { jvalue args[2]; args[0].l = carbonRow; args[1].i = ordinal; - return (jobjectArray) jniEnv->CallStaticObjectMethodA(rowUtilClass, getArrayId, args); + jobjectArray result = (jobjectArray) jniEnv->CallStaticObjectMethodA(rowUtilClass, getArrayId, args); + if (jniEnv->ExceptionCheck()) { + throw jniEnv->ExceptionOccurred(); + } + return result; +} + +void CarbonRow::close() { + jniEnv->DeleteLocalRef(rowUtilClass); + jniEnv->DeleteLocalRef(carbonRow); } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/src/CarbonRow.h ---------------------------------------------------------------------- diff --git a/store/CSDK/src/CarbonRow.h b/store/CSDK/src/CarbonRow.h index c57f826..e715299 100644 --- a/store/CSDK/src/CarbonRow.h +++ b/store/CSDK/src/CarbonRow.h @@ -154,4 +154,10 @@ public: * @return jobjectArray data type data */ jobjectArray getArray(int ordinal); + + /** + * delete data and release + * @return + */ + void close(); }; http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/src/CarbonWriter.cpp ---------------------------------------------------------------------- diff --git a/store/CSDK/src/CarbonWriter.cpp b/store/CSDK/src/CarbonWriter.cpp index 6619e33..2a4bcc7 100644 --- a/store/CSDK/src/CarbonWriter.cpp +++ b/store/CSDK/src/CarbonWriter.cpp @@ -164,4 +164,7 @@ void CarbonWriter::close() { if (jniEnv->ExceptionCheck()) { throw jniEnv->ExceptionOccurred(); } + jniEnv->DeleteLocalRef(carbonWriterBuilderObject); + jniEnv->DeleteLocalRef(carbonWriterObject); + jniEnv->DeleteLocalRef(carbonWriter); } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/CSDK/test/main.cpp ---------------------------------------------------------------------- diff --git a/store/CSDK/test/main.cpp b/store/CSDK/test/main.cpp index 44a7c69..5ab3976 100644 --- a/store/CSDK/test/main.cpp +++ b/store/CSDK/test/main.cpp @@ -72,10 +72,16 @@ JNIEnv *initJVM() { * @param arr array */ void printArray(JNIEnv *env, jobjectArray arr) { + if (env->ExceptionCheck()) { + throw env->ExceptionOccurred(); + } jsize length = env->GetArrayLength(arr); int j = 0; for (j = 0; j < length; j++) { jobject element = env->GetObjectArrayElement(arr, j); + if (env->ExceptionCheck()) { + throw env->ExceptionOccurred(); + } char *str = (char *) env->GetStringUTFChars((jstring) element, JNI_FALSE); printf("%s\t", str); } @@ -102,27 +108,66 @@ void printBoolean(jboolean bool1) { * @param env JNIEnv * @param reader CarbonReader object */ +void printResultWithException(JNIEnv *env, CarbonReader reader) { + try { + CarbonRow carbonRow(env); + while (reader.hasNext()) { + jobject row = reader.readNextRow(); + carbonRow.setCarbonRow(row); + printf("%s\t", carbonRow.getString(1)); + printf("%d\t", carbonRow.getInt(1)); + printf("%ld\t", carbonRow.getLong(2)); + printf("%s\t", carbonRow.getVarchar(1)); + printArray(env, carbonRow.getArray(0)); + printf("%d\t", carbonRow.getShort(5)); + printf("%d\t", carbonRow.getInt(6)); + printf("%ld\t", carbonRow.getLong(7)); + printf("%lf\t", carbonRow.getDouble(8)); + printBoolean(carbonRow.getBoolean(9)); + printf("%s\t", carbonRow.getDecimal(9)); + printf("%f\t", carbonRow.getFloat(11)); + printf("\n"); + env->DeleteLocalRef(row); + } + reader.close(); + carbonRow.close(); + } catch (jthrowable e) { + env->ExceptionDescribe(); + } +} + +/** + * print result of reading data + * + * @param env JNIEnv + * @param reader CarbonReader object + */ void printResult(JNIEnv *env, CarbonReader reader) { - CarbonRow carbonRow(env); - while (reader.hasNext()) { - jobject row = reader.readNextRow(); - carbonRow.setCarbonRow(row); - printf("%s\t", carbonRow.getString(0)); - printf("%d\t", carbonRow.getInt(1)); - printf("%ld\t", carbonRow.getLong(2)); - printf("%s\t", carbonRow.getVarchar(3)); - printArray(env, carbonRow.getArray(4)); - printf("%d\t", carbonRow.getShort(5)); - printf("%d\t", carbonRow.getInt(6)); - printf("%ld\t", carbonRow.getLong(7)); - printf("%lf\t", carbonRow.getDouble(8)); - printBoolean(carbonRow.getBoolean(9)); - printf("%s\t", carbonRow.getDecimal(10)); - printf("%f\t", carbonRow.getFloat(11)); - printf("\n"); - env->DeleteLocalRef(row); + try { + CarbonRow carbonRow(env); + while (reader.hasNext()) { + jobject row = reader.readNextRow(); + carbonRow.setCarbonRow(row); + printf("%s\t", carbonRow.getString(0)); + printf("%d\t", carbonRow.getInt(1)); + printf("%ld\t", carbonRow.getLong(2)); + printf("%s\t", carbonRow.getVarchar(3)); + printArray(env, carbonRow.getArray(4)); + printf("%d\t", carbonRow.getShort(5)); + printf("%d\t", carbonRow.getInt(6)); + printf("%ld\t", carbonRow.getLong(7)); + printf("%lf\t", carbonRow.getDouble(8)); + printBoolean(carbonRow.getBoolean(9)); + printf("%s\t", carbonRow.getDecimal(10)); + printf("%f\t", carbonRow.getFloat(11)); + printf("\n"); + env->DeleteLocalRef(row); + } + carbonRow.close(); + reader.close(); + } catch (jthrowable e) { + env->ExceptionDescribe(); } - reader.close(); } /** @@ -159,6 +204,30 @@ bool readSchema(JNIEnv *env, char *Path, bool validateSchema) { } /** + * test the exception when carbonRow with wrong index. + * + * @param env jni env + * @return + */ +bool tryCarbonRowException(JNIEnv *env, char *path) { + printf("\nRead data from local without projection:\n"); + + CarbonReader carbonReaderClass; + try { + carbonReaderClass.builder(env, path); + } catch (runtime_error e) { + printf("\nget exception fro builder and throw\n"); + throw e; + } + try { + carbonReaderClass.build(); + } catch (jthrowable e) { + env->ExceptionDescribe(); + } + printResultWithException(env, carbonReaderClass); +} + +/** * test read data from local disk, without projection * * @param env jni env @@ -251,6 +320,7 @@ void testReadNextRow(JNIEnv *env, char *path, int printNum, char **argv, int arg printf("total line is: %d,\t build time is: %lf s,\tread time is %lf s, average speed is %lf records/s ", i, buildTime, time / 1000000.0, i / (time / 1000000.0)); carbonReaderClass.close(); + carbonRow.close(); } catch (jthrowable) { env->ExceptionDescribe(); } @@ -353,6 +423,7 @@ void testReadNextBatchRow(JNIEnv *env, char *path, int batchSize, int printNum, printf("total line is: %d,\t build time is: %lf s,\tread time is %lf s, average speed is %lf records/s ", i, buildTime, time / 1000000.0, i / (time / 1000000.0)); carbonReaderClass.close(); + carbonRow.close(); } /** @@ -410,6 +481,7 @@ bool readFromLocalWithProjection(JNIEnv *env, char *path) { } reader.close(); + carbonRow.close(); } @@ -566,6 +638,7 @@ bool testWriteData(JNIEnv *env, char *path, int argc, char *argv[]) { env->DeleteLocalRef(row); } carbonReader.close(); + carbonRow.close(); } catch (jthrowable ex) { env->ExceptionDescribe(); env->ExceptionClear(); @@ -604,36 +677,39 @@ int main(int argc, char *argv[]) { // init jvm JNIEnv *env; env = initJVM(); - char *S3WritePath = "s3a://sdk/WriterOutput/carbondata2"; - char *S3ReadPath = "s3a://sdk/WriterOutput/carbondata"; + char *S3WritePath = "s3a://csdk/WriterOutput/carbondata2"; + char *S3ReadPath = "s3a://csdk/WriterOutput/carbondata"; char *smallFilePath = "../../../../resources/carbondata"; char *path = "../../../../../../../Downloads/carbon-data-big/dir2"; - char *S3Path = "s3a://sdk/ges/i400bs128"; + char *S3Path = "s3a://csdk/bigData/i400bs128"; if (argc > 3) { // TODO: need support read schema from S3 in the future testWriteData(env, S3WritePath, 4, argv); readFromS3(env, S3ReadPath, argv); + testReadNextRow(env, S3Path, 100000, argv, 4, false); testReadNextRow(env, S3Path, 100000, argv, 4, true); testReadNextBatchRow(env, S3Path, 100000, 100000, argv, 4, false); testReadNextBatchRow(env, S3Path, 100000, 100000, argv, 4, true); } else { tryCatchException(env); + tryCarbonRowException(env, smallFilePath); testCarbonProperties(env); testWriteData(env, "./data", 1, argv); testWriteData(env, "./data", 1, argv); readFromLocalWithoutProjection(env, smallFilePath); readFromLocalWithProjection(env, smallFilePath); + readSchema(env, path, false); + readSchema(env, path, true); + int batch = 32000; int printNum = 32000; testReadNextRow(env, path, printNum, argv, 0, true); testReadNextRow(env, path, printNum, argv, 0, false); testReadNextBatchRow(env, path, batch, printNum, argv, 0, true); testReadNextBatchRow(env, path, batch, printNum, argv, 0, false); - readSchema(env, path, false); - readSchema(env, path, true); } (jvm)->DestroyJavaVM(); http://git-wip-us.apache.org/repos/asf/carbondata/blob/0fa0a96c/store/sdk/src/main/java/org/apache/carbondata/sdk/file/RowUtil.java ---------------------------------------------------------------------- diff --git a/store/sdk/src/main/java/org/apache/carbondata/sdk/file/RowUtil.java b/store/sdk/src/main/java/org/apache/carbondata/sdk/file/RowUtil.java index b7a594f..fdf3cfc 100644 --- a/store/sdk/src/main/java/org/apache/carbondata/sdk/file/RowUtil.java +++ b/store/sdk/src/main/java/org/apache/carbondata/sdk/file/RowUtil.java @@ -70,7 +70,12 @@ public class RowUtil implements Serializable { * @return array data type data */ public static Object[] getArray(Object[] data, int ordinal) { - return (Object[]) data[ordinal]; + Object object = data[ordinal]; + if (object instanceof Object[]) { + return (Object[]) data[ordinal]; + } else { + throw new IllegalArgumentException("It's not an array in ordinal of data."); + } } /**
