jacob-project icon indicating copy to clipboard operation
jacob-project copied to clipboard

Memory leaks and incorrect Java exception throws in SafeArray.cpp

Open EJP286CRSKW opened this issue 1 year ago • 1 comments


/* PLEASE NOTE THE LINE: 
	jint *jIndices = env->GetIntArrayElements(indices, NULL);
	which I added to replace "long idx[2] = {i,j};" from the 2D case.
	Not sure if this is correct. Also, check that I am doing the null test and
	dimension test correctly.
	Would really like to call     env->ReleaseIntArrayElements(indices, jIndices, NULL);    
	in here but I can't get it to work
*/

#define GETNDCODE(varType, varAccess, jtyp) \
  SAFEARRAY *sa = extractSA(env, _this); \
  if (!sa) { \
    ThrowComFail(env, "safearray object corrupted", -1); \
    return NULL; \
  } \
  jint *jIndices = env->GetIntArrayElements(indices, NULL); \
  if (!jIndices) { \
    ThrowComFail(env, "null indices array", -1); \
    return NULL; \
  } \
  if (SafeArrayGetDim(sa) != env->GetArrayLength(indices)) { \
    ThrowComFail(env, "safearray and indices array have different dimensions", -1); \
    return NULL; \
  } \
  VARTYPE vt; \
  SafeArrayGetVartype(sa, &vt); \
  if (vt == VT_VARIANT) { \
    VARIANT v; \
    VariantInit(&v); \
    SafeArrayGetElement(sa, jIndices, (void*) &v); \
    if (FAILED(VariantChangeType(&v, &v, 0, varType))) { \
      ThrowComFail(env, "VariantChangeType failed", -1); \
      return NULL; \
    } \
    return (jtyp)varAccess(&v); \
  } else if (vt == varType) { \
    jtyp jc; \
    SafeArrayGetElement(sa, jIndices, (void*) &jc); \
    return jc; \
  } else { \
    return NULL; \
  } 
  1. The test (!jIndices) does not indicate a null indices array. It indicates out of memory when retrieving the native values. At this point an OutOfMemoryError is already pending and it is incorrect to throw another exception: this will crash the JVM. The ThrowComFail() call in this block must be deleted. The only thing that can be done at this point is either ExceptionClear(), ExceptionDescribe(), or just return.
  2. A prior check for (indices == NULL) is required, which should throw NullPointerException.
  3. env->ReleaseIntArrayElements(indices, jIndices, 0) must be called before all the following return statements in this macro. Note that the third argument is an integer, not a pointer. Otherwise jIndices constitutes a memory leak. The comment says the author was unable to get this working, but I did, so I can't see what the problem was.

The same observations apply to the SETNDCODE() macro.

EJP286CRSKW avatar Nov 06 '23 03:11 EJP286CRSKW

Code sample or PR?

freemansoft avatar Mar 18 '24 03:03 freemansoft