From 8275eb09d99e6d79085ff744401a51a6372c4bcf Mon Sep 17 00:00:00 2001 From: xubo245 Date: Mon, 19 Nov 2018 22:48:31 +0800 Subject: [PATCH] [CARBONDATA-3108][CARBONDATA-3044] Fix the error of jvm will crash when CarbonRow use wrong index number in CSDK 1. fix the error 2. delete/release the data after running 3. init the variable to NULL change the return value optimized --- 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 ++++++++++++++---- .../apache/carbondata/sdk/file/RowUtil.java | 7 +- 7 files changed, 177 insertions(+), 35 deletions(-) diff --git a/store/CSDK/src/CarbonReader.cpp b/store/CSDK/src/CarbonReader.cpp index 8375b73f5a7..8ee65d08e74 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 diff --git a/store/CSDK/src/CarbonReader.h b/store/CSDK/src/CarbonReader.h index 246e24bb475..9a1daebc1aa 100644 --- a/store/CSDK/src/CarbonReader.h +++ b/store/CSDK/src/CarbonReader.h @@ -45,6 +45,11 @@ class CarbonReader { */ 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 diff --git a/store/CSDK/src/CarbonRow.cpp b/store/CSDK/src/CarbonRow.cpp index f7066eced48..45cf8f60445 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 diff --git a/store/CSDK/src/CarbonRow.h b/store/CSDK/src/CarbonRow.h index c57f8266ec2..e71529955bc 100644 --- a/store/CSDK/src/CarbonRow.h +++ b/store/CSDK/src/CarbonRow.h @@ -154,4 +154,10 @@ class CarbonRow { * @return jobjectArray data type data */ jobjectArray getArray(int ordinal); + + /** + * delete data and release + * @return + */ + void close(); }; diff --git a/store/CSDK/src/CarbonWriter.cpp b/store/CSDK/src/CarbonWriter.cpp index 6619e339101..2a4bcc725d1 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 diff --git a/store/CSDK/test/main.cpp b/store/CSDK/test/main.cpp index 44a7c69a23e..5ab3976daa9 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); } @@ -96,6 +102,40 @@ void printBoolean(jboolean bool1) { } } +/** + * print result of reading data + * + * @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 * @@ -103,26 +143,31 @@ void printBoolean(jboolean bool1) { * @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(); } /** @@ -158,6 +203,30 @@ bool readSchema(JNIEnv *env, char *Path, bool validateSchema) { return true; } +/** + * 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 * @@ -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(); 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 b7a594fde30..fdf3cfc7f88 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 static long getLong(Object[] data, int ordinal) { * @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."); + } } /**