freemansoft / jacob-project

JACOB is a JAVA-COM Bridge that allows you to call COM Automation components from Java. It uses JNI to make native calls to the COM libraries. JACOB runs on x86 and x64 environments supporting 32 bit and 64 bit JVMs. This repository was migrated from Sourceforge 2020 Sep
GNU Lesser General Public License v2.1
521 stars 117 forks source link

Memory leak in Dispatch.cpp #36

Closed EJP286CRSKW closed 6 months ago

EJP286CRSKW commented 1 year ago

There is a memory leak around the call to name2ID(), in the case where it fails. Code reads:

  if (name != NULL) 
  {
    const char *nm = env->GetStringUTFChars(name, NULL);
    HRESULT hr;
    if (FAILED(hr = name2ID(pIDispatch, nm, (long *)&dispID, lcid))) {
      char buf[1024];
      sprintf_s(buf, 1024, "Can't map name to dispid: %s", nm);
      ThrowComFail(env, buf, -1);
      return NULL;
    }
    env->ReleaseStringUTFChars(name, nm);
  }

It should read:

  if (name != NULL) 
  {
    const char *nm = env->GetStringUTFChars(name, NULL);
    HRESULT hr = name2ID(pIDispatch, nm, (long *)&dispID, lcid));
    env->ReleaseStringUTFChars(name, nm);
    if (FAILED(hr)) {
      char buf[1024];
      sprintf_s(buf, 1024, "Can't map name to dispid: %s", nm);
      ThrowComFail(env, buf, -1);
      return NULL;
    }
  }
freemansoft commented 1 year ago

You are saying that the leak is because the release string is skipped if it fails because the throw() exits the function before ReleaseString is called?

Ugh. I'll have to look at putting my dev environment back together.

freemansoft commented 1 year ago

How did you find this?

EJP286CRSKW commented 1 year ago

The ReleaseStringUTFChars() is skipped because of the premature return NULL; in the failure block.

I found it by reading the code.

freemansoft commented 6 months ago

I think

    HRESULT hr = name2ID(pIDispatch, nm, (long *)&dispID, lcid));

Should be

    HRESULT hr = name2ID(pIDispatch, nm, (long *)&dispID, lcid);

https://github.com/freemansoft/jacob-project/commit/585338153351d24bb17afae4c2402cd143f1362f

EJP286CRSKW commented 6 months ago

Correct, well spotted.