Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leaks and incorrect Java exception throws in SafeArray.cpp #47

Open
EJP286CRSKW opened this issue Nov 6, 2023 · 1 comment
Open

Comments

@EJP286CRSKW
Copy link

EJP286CRSKW commented Nov 6, 2023


/* 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.

@freemansoft
Copy link
Owner

Code sample or PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants