diff --git a/CHANGELOG.md b/CHANGELOG.md
index 568bbbd2d1078face20f0a61d1e40bafa144e633..4b87afff0798b5158b1ac5ba245fed8ba566d01b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
## 1.4.2
+* Fixed Potential JVM crash when using multi-dimensional arrays (> 1D)
+
## 1.4.1
* Fixed exception handling when calling Java method OpenCLDevice.configure() to not hide exceptions
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
index 0997bd20ea7165f088861dc993dfecdb75a993ac..a272aa362993093996743da4250213ee4de6ebfa 100644
--- a/CONTRIBUTORS.md
+++ b/CONTRIBUTORS.md
@@ -31,3 +31,5 @@ Below are some of the specific details of various contributions.
* Luis Mendes submitted local arrays handling 1D and ND, to cope with arrays resizing across kernel executions
* Luis Mendes submitted #107 aparapi now supports efficient execution on discrete GPU and other devices
* Luis Mendes submitted - Support for OpenCLDevice configurator/configure API
+* Luis Mendes - Fix for Issue #153 Potential JVM crash when using multi-dimensional arrays(>1D) (refs #153)
+* Luis Mendes - Code refactorization for AparapiBuffer and ArrayBuffer classes improving OO concerns separation
\ No newline at end of file
diff --git a/src/cpp/runKernel/Aparapi.cpp b/src/cpp/runKernel/Aparapi.cpp
index 02d967620ae5982dc488becd453553f145b97908..807bf81e437612ae98e6158282c4b1e34c488c34 100644
--- a/src/cpp/runKernel/Aparapi.cpp
+++ b/src/cpp/runKernel/Aparapi.cpp
@@ -284,39 +284,8 @@ jint updateNonPrimitiveReferences(JNIEnv *jenv, jobject jobj, JNIContext* jniCon
if (config->isVerbose()){
fprintf(stderr, "Resync javaArray for %s: %p %p\n", arg->name, newRef, arg->arrayBuffer->javaArray);
}
- // Free previous ref if any
- if (arg->arrayBuffer->javaArray != NULL) {
- jenv->DeleteWeakGlobalRef((jweak) arg->arrayBuffer->javaArray);
- if (config->isVerbose()){
- fprintf(stderr, "DeleteWeakGlobalRef for %s: %p\n", arg->name, arg->arrayBuffer->javaArray);
- }
- }
-
- // need to free opencl buffers, run will reallocate later
- if (arg->arrayBuffer->mem != 0) {
- //fprintf(stderr, "-->releaseMemObject[%d]\n", i);
- if (config->isTrackingOpenCLResources()){
- memList.remove(arg->arrayBuffer->mem,__LINE__, __FILE__);
- }
- status = clReleaseMemObject((cl_mem)arg->arrayBuffer->mem);
- //fprintf(stderr, "<--releaseMemObject[%d]\n", i);
- if(status != CL_SUCCESS) throw CLException(status, "clReleaseMemObject()");
- arg->arrayBuffer->mem = (cl_mem)0;
- }
-
- arg->arrayBuffer->addr = NULL;
-
- // Capture new array ref from the kernel arg object
-
- if (newRef != NULL) {
- arg->arrayBuffer->javaArray = (jarray)jenv->NewWeakGlobalRef((jarray)newRef);
- if (config->isVerbose()){
- fprintf(stderr, "NewWeakGlobalRef for %s, set to %p\n", arg->name,
- arg->arrayBuffer->javaArray);
- }
- } else {
- arg->arrayBuffer->javaArray = NULL;
- }
+ // Free previous ref if any and allocate new
+ arg->arrayBuffer->replaceJavaArray(jenv, arg, newRef);
// Save the lengthInBytes which was set on the java side
int lengthInBytes = arg->getSizeInBytes(jenv);
@@ -413,7 +382,6 @@ void updateArray(JNIEnv* jenv, JNIContext* jniContext, KernelArg* arg, int& argP
}
void updateBuffer(JNIEnv* jenv, JNIContext* jniContext, KernelArg* arg, int& argPos, int argIdx) {
-
AparapiBuffer* buffer = arg->aparapiBuffer;
cl_int status = CL_SUCCESS;
cl_uint mask = 0;
diff --git a/src/cpp/runKernel/AparapiBuffer.cpp b/src/cpp/runKernel/AparapiBuffer.cpp
index 93ab2a7a5f3efbbdc78d2a8cfba449286f357fb3..bef148606005bfbf447346a58cc1fb08a80d67e6 100644
--- a/src/cpp/runKernel/AparapiBuffer.cpp
+++ b/src/cpp/runKernel/AparapiBuffer.cpp
@@ -53,6 +53,7 @@
#define APARAPIBUFFER_SOURCE
#include "AparapiBuffer.h"
#include "KernelArg.h"
+#include "List.h"
AparapiBuffer::AparapiBuffer():
javaObject((jobject) 0),
@@ -67,20 +68,6 @@ AparapiBuffer::AparapiBuffer():
}
}
-AparapiBuffer::AparapiBuffer(void* _data, cl_uint* _lens, cl_uint _numDims, long _lengthInBytes, jobject _javaObject) :
- data(_data),
- numDims(_numDims),
- lengthInBytes(_lengthInBytes),
- javaObject(_javaObject),
- mem((cl_mem) 0),
- memMask((cl_uint)0)
-{
- for(int i = 0; i < _numDims; i++) {
- lens[i] = _lens[i];
- }
- computeOffsets();
-}
-
void AparapiBuffer::computeOffsets() {
for(int i = 0; i < numDims; i++) {
offsets[i] = 1;
@@ -94,6 +81,63 @@ jobject AparapiBuffer::getJavaObject(JNIEnv* env, KernelArg* arg) const {
return JNIHelper::getInstanceField<jobject>(env, arg->javaArg, "javaBuffer", ObjectClassArg);
}
+void AparapiBuffer::replaceJavaObject(JNIEnv* jenv, KernelArg* arg, jobject newJavaObject) {
+ cl_int status = CL_SUCCESS;
+ if (javaObject != NULL) {
+ if (config->isVerbose()){
+ fprintf(stderr, "DeleteWeakGlobalRef for %s: %p\n", arg->name, javaObject);
+ }
+ jenv->DeleteWeakGlobalRef((jweak) javaObject);
+ javaObject = 0;
+ }
+
+ // need to free opencl buffers, run will reallocate later
+ if (mem != 0) {
+ //fprintf(stderr, "-->releaseMemObject[%d]\n", i);
+ if (config->isTrackingOpenCLResources()){
+ memList.remove(mem,__LINE__, __FILE__);
+ }
+ status = clReleaseMemObject((cl_mem)mem);
+ //fprintf(stderr, "<--releaseMemObject[%d]\n", i);
+ if(status != CL_SUCCESS) throw CLException(status, "clReleaseMemObject()");
+ mem = (cl_mem)0;
+ }
+
+ // Capture new array ref from the kernel arg object
+ if (newJavaObject != 0) {
+ javaObject = (jarray)jenv->NewWeakGlobalRef((jarray)newJavaObject);
+ if (config->isVerbose()){
+ fprintf(stderr, "NewWeakGlobalRef for %s, set to %p\n", arg->name,
+ javaObject);
+ }
+ } else {
+ javaObject = 0;
+ }
+}
+
+void AparapiBuffer::deleteJavaObject(JNIEnv* jenv, KernelArg* arg) {
+ cl_int status = CL_SUCCESS;
+ if (javaObject != 0) {
+ if (config->isVerbose()){
+ fprintf(stderr, "DeleteWeakGlobalRef for %s: %p\n", arg->name, javaObject);
+ }
+ jenv->DeleteWeakGlobalRef((jweak) javaObject);
+ javaObject = 0;
+ }
+
+ // need to free opencl buffers, run will reallocate later
+ if (mem != 0) {
+ //fprintf(stderr, "-->releaseMemObject[%d]\n", i);
+ if (config->isTrackingOpenCLResources()){
+ memList.remove(mem,__LINE__, __FILE__);
+ }
+ status = clReleaseMemObject((cl_mem)mem);
+ //fprintf(stderr, "<--releaseMemObject[%d]\n", i);
+ if(status != CL_SUCCESS) throw CLException(status, "clReleaseMemObject()");
+ mem = (cl_mem)0;
+ }
+}
+
void AparapiBuffer::getMinimalParams(JNIEnv *env, KernelArg* arg, cl_uint &numDims, cl_uint dims[], int &lengthInBytes) const {
numDims = JNIHelper::getInstanceField<jint>(env, arg->javaArg, "numDims", IntArg);
jobjectArray jObjArray = (jobjectArray)getJavaObject(env, arg);
@@ -155,14 +199,14 @@ void AparapiBuffer::flatten(JNIEnv* env, KernelArg* arg) {
}
-void AparapiBuffer::buildBuffer(void* _data, cl_uint* _dims, cl_uint _numDims, long _lengthInBytes, jobject _javaObject) {
+void AparapiBuffer::buildBuffer(JNIEnv* env, KernelArg* arg, void* _data, cl_uint* _dims, cl_uint _numDims, long _lengthInBytes, jobject _javaObject) {
data = _data;
numDims = _numDims;
lengthInBytes = _lengthInBytes;
- javaObject = _javaObject;
for(int i = 0; i < _numDims; i++) {
lens[i] = _dims[i];
}
+ replaceJavaObject(env, arg, _javaObject);
computeOffsets();
}
@@ -212,7 +256,7 @@ void AparapiBuffer::flattenBoolean2D(JNIEnv* env, KernelArg* arg) {
// env->DeleteLocalRef(jArray);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenByte2D(JNIEnv* env, KernelArg* arg) {
@@ -258,7 +302,7 @@ void AparapiBuffer::flattenByte2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseByteArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenShort2D(JNIEnv* env, KernelArg* arg) {
@@ -304,7 +348,7 @@ void AparapiBuffer::flattenShort2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseShortArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenInt2D(JNIEnv* env, KernelArg* arg) {
@@ -350,7 +394,7 @@ void AparapiBuffer::flattenInt2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseIntArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenLong2D(JNIEnv* env, KernelArg* arg) {
@@ -396,7 +440,7 @@ void AparapiBuffer::flattenLong2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseLongArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenFloat2D(JNIEnv* env, KernelArg* arg) {
@@ -442,7 +486,7 @@ void AparapiBuffer::flattenFloat2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseFloatArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
void AparapiBuffer::flattenDouble2D(JNIEnv* env, KernelArg* arg) {
@@ -488,7 +532,7 @@ void AparapiBuffer::flattenDouble2D(JNIEnv* env, KernelArg* arg) {
env->ReleaseDoubleArrayElements(jArray, elems, 0);
}
- buildBuffer((void*)array, dims, 2, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 2, bitSize, javaBuffer);
}
@@ -553,7 +597,7 @@ void AparapiBuffer::flattenBoolean3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenByte3D(JNIEnv* env, KernelArg* arg) {
@@ -617,7 +661,7 @@ void AparapiBuffer::flattenByte3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenShort3D(JNIEnv* env, KernelArg* arg) {
@@ -681,7 +725,7 @@ void AparapiBuffer::flattenShort3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenInt3D(JNIEnv* env, KernelArg* arg) {
@@ -745,7 +789,7 @@ void AparapiBuffer::flattenInt3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenLong3D(JNIEnv* env, KernelArg* arg) {
@@ -809,7 +853,7 @@ void AparapiBuffer::flattenLong3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenFloat3D(JNIEnv* env, KernelArg* arg) {
@@ -873,7 +917,7 @@ void AparapiBuffer::flattenFloat3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::flattenDouble3D(JNIEnv* env, KernelArg* arg) {
@@ -937,13 +981,12 @@ void AparapiBuffer::flattenDouble3D(JNIEnv* env, KernelArg* arg) {
}
}
- buildBuffer((void*)array, dims, 3, bitSize, javaBuffer);
+ buildBuffer(env, arg, (void*)array, dims, 3, bitSize, javaBuffer);
}
void AparapiBuffer::inflate(JNIEnv* env, KernelArg* arg) {
- javaObject = JNIHelper::getInstanceField<jobject>(env, arg->javaArg, "javaBuffer", ObjectClassArg);
if(numDims == 2 && arg->isBoolean()) {
AparapiBuffer::inflateBoolean2D(env, arg);
} else if(numDims == 2 && arg->isByte()) {
diff --git a/src/cpp/runKernel/AparapiBuffer.h b/src/cpp/runKernel/AparapiBuffer.h
index 2429d14b89cc21aafdbebf17b611801b6f0568f8..a345148e4b41e9102f347ce69182161fc194cc75 100644
--- a/src/cpp/runKernel/AparapiBuffer.h
+++ b/src/cpp/runKernel/AparapiBuffer.h
@@ -85,12 +85,12 @@ private:
return (type&com_aparapi_internal_jni_KernelRunnerJNI_ARG_SHORT);
}
- void buildBuffer(void* _data, cl_uint* _dims, cl_uint _numDims, long _lengthInBytes, jobject _javaObject);
+ void buildBuffer(JNIEnv* env, KernelArg* arg, void* _data, cl_uint* _dims, cl_uint _numDims, long _lengthInBytes, jobject _javaObject);
void computeOffsets();
public:
- static int const MAX_ND_DIMS = 3;
+ static int const MAX_ND_DIMS = 3;
jobject javaObject; // The java array that this arg is mapped to
cl_uint numDims; // number of dimensions of the object (array lengths for ND arrays)
@@ -153,6 +153,8 @@ public:
void inflateDouble3D(JNIEnv *env, KernelArg* arg);
jobject getJavaObject(JNIEnv* env, KernelArg* arg) const;
+ void replaceJavaObject(JNIEnv* env, KernelArg* arg, jobject newJavaObject);
+ void deleteJavaObject(JNIEnv* env, KernelArg* arg);
};
#endif // ARRAYBUFFER_H
diff --git a/src/cpp/runKernel/ArrayBuffer.cpp b/src/cpp/runKernel/ArrayBuffer.cpp
index 42300d57d78ee2e7bee54a05913b7f5ca9ec4420..895220f2ab466a61c459f2559ba99412ef059de0 100644
--- a/src/cpp/runKernel/ArrayBuffer.cpp
+++ b/src/cpp/runKernel/ArrayBuffer.cpp
@@ -53,6 +53,8 @@
#define ARRAYBUFFER_SOURCE
#include "ArrayBuffer.h"
#include "KernelArg.h"
+#include "List.h"
+
ArrayBuffer::ArrayBuffer():
javaArray((jobject) 0),
@@ -86,5 +88,69 @@ void ArrayBuffer::getMinimalParams(JNIEnv *jenv, KernelArg *arg, cl_uint& arrayE
void ArrayBuffer::syncMinimalParams(JNIEnv *jenv, KernelArg *arg) {
length = JNIHelper::getInstanceField<jint>(jenv, arg->javaArg, "numElements", IntArg);
- lengthInBytes = jenv->GetIntField(arg->javaArg, KernelArg::getSizeInBytesFieldID());
+ lengthInBytes = jenv->GetIntField(arg->javaArg, KernelArg::getSizeInBytesFieldID());
+}
+
+void ArrayBuffer::replaceJavaArray(JNIEnv *jenv, KernelArg *arg, jarray newRef) {
+ cl_int status = CL_SUCCESS;
+ if (javaArray != NULL) {
+ if (config->isVerbose()){
+ fprintf(stderr, "DeleteWeakGlobalRef for %s: %p\n", arg->name, javaArray);
+ }
+ jenv->DeleteWeakGlobalRef((jweak) javaArray);
+ javaArray = 0;
+ }
+
+ // need to free opencl buffers, run will reallocate later
+ if (mem != 0) {
+ //fprintf(stderr, "-->releaseMemObject[%d]\n", i);
+ if (config->isTrackingOpenCLResources()){
+ memList.remove(mem,__LINE__, __FILE__);
+ }
+ status = clReleaseMemObject((cl_mem)mem);
+ //fprintf(stderr, "<--releaseMemObject[%d]\n", i);
+ CLException::checkCLError(status, "clReleaseMemObject()");
+ if(status != CL_SUCCESS) throw CLException(status, "clReleaseMemObject()");
+ mem = (cl_mem)0;
+ }
+
+ addr = 0;
+
+ // Capture new array ref from the kernel arg object
+ if (newRef != 0) {
+ javaArray = (jarray)jenv->NewWeakGlobalRef((jarray)newRef);
+ if (config->isVerbose()){
+ fprintf(stderr, "NewWeakGlobalRef for %s, set to %p\n", arg->name,
+ javaArray);
+ }
+ } else {
+ javaArray = 0;
+ }
+}
+
+
+void ArrayBuffer::deleteJavaArray(JNIEnv *jenv, KernelArg *arg) {
+ cl_int status = CL_SUCCESS;
+ if (javaArray != NULL) {
+ if (config->isVerbose()){
+ fprintf(stderr, "DeleteWeakGlobalRef for %s: %p\n", arg->name, ArrayBuffer::javaArray);
+ }
+ jenv->DeleteWeakGlobalRef((jweak) ArrayBuffer::javaArray);
+ javaArray=0;
+ }
+
+ // need to free opencl buffers, run will reallocate later
+ if (mem != 0) {
+ //fprintf(stderr, "-->releaseMemObject[%d]\n", i);
+ if (config->isTrackingOpenCLResources()){
+ memList.remove(mem,__LINE__, __FILE__);
+ }
+ status = clReleaseMemObject((cl_mem)mem);
+ //fprintf(stderr, "<--releaseMemObject[%d]\n", i);
+ CLException::checkCLError(status, "clReleaseMemObject()");
+ if(status != CL_SUCCESS) throw CLException(status, "clReleaseMemObject()");
+ mem = (cl_mem)0;
+ }
+
+ addr = 0;
}
diff --git a/src/cpp/runKernel/ArrayBuffer.h b/src/cpp/runKernel/ArrayBuffer.h
index 26cdbe567c1ace9ba48c7ada1017237e871ef21f..1be98a07d4f59ea0d90df9303735d6effba5d200 100644
--- a/src/cpp/runKernel/ArrayBuffer.h
+++ b/src/cpp/runKernel/ArrayBuffer.h
@@ -78,6 +78,8 @@ class ArrayBuffer{
void pin(JNIEnv *jenv);
void getMinimalParams(JNIEnv *jenv, KernelArg *arg, cl_uint& arrayElements, int &sizeInBytes);
void syncMinimalParams(JNIEnv *jenv, KernelArg *arg);
+ void replaceJavaArray(JNIEnv *jenv, KernelArg *arg, jarray newRef);
+ void deleteJavaArray(JNIEnv *jenv, KernelArg *arg);
};
#endif // ARRAYBUFFER_H
diff --git a/src/cpp/runKernel/JNIContext.cpp b/src/cpp/runKernel/JNIContext.cpp
index 363f388aa5f7afb43d9c0b0375defcd06e8d3689..569fc9a6864c90ab812e725f3b73c3e26aa2aa95 100644
--- a/src/cpp/runKernel/JNIContext.cpp
+++ b/src/cpp/runKernel/JNIContext.cpp
@@ -86,35 +86,13 @@ void JNIContext::dispose(JNIEnv *jenv, Config* config) {
if (!arg->isPrimitive()){
if (arg->isArray()) {
if (arg->arrayBuffer != NULL){
- if (arg->arrayBuffer->mem != 0){
- if (config->isTrackingOpenCLResources()){
- memList.remove((cl_mem)arg->arrayBuffer->mem, __LINE__, __FILE__);
- }
- status = clReleaseMemObject((cl_mem)arg->arrayBuffer->mem);
- //fprintf(stdout, "dispose arg %d %0lx\n", i, arg->arrayBuffer->mem);
- CLException::checkCLError(status, "clReleaseMemObject()");
- arg->arrayBuffer->mem = (cl_mem)0;
- }
- if (arg->arrayBuffer->javaArray != NULL) {
- jenv->DeleteWeakGlobalRef((jweak) arg->arrayBuffer->javaArray);
- }
+ arg->arrayBuffer->deleteJavaArray(jenv, arg);
delete arg->arrayBuffer;
arg->arrayBuffer = NULL;
}
} else if (arg->isAparapiBuffer()) {
if (arg->aparapiBuffer != NULL){
- if (arg->aparapiBuffer->mem != 0){
- if (config->isTrackingOpenCLResources()){
- memList.remove((cl_mem)arg->aparapiBuffer->mem, __LINE__, __FILE__);
- }
- status = clReleaseMemObject((cl_mem)arg->aparapiBuffer->mem);
- //fprintf(stdout, "dispose arg %d %0lx\n", i, arg->aparapiBuffer->mem);
- CLException::checkCLError(status, "clReleaseMemObject()");
- arg->aparapiBuffer->mem = (cl_mem)0;
- }
- if (arg->aparapiBuffer->javaObject != NULL) {
- jenv->DeleteWeakGlobalRef((jweak) arg->aparapiBuffer->javaObject);
- }
+ arg->aparapiBuffer->deleteJavaObject(jenv, arg);
delete arg->aparapiBuffer;
arg->aparapiBuffer = NULL;
}