//
you're reading...
BSTR, COM, Programming Issues/Tips

Misunderstanding IDL Parameter Direction Leading to BSTR Memory Leakage.

1. Introduction.

1.1 Just yesterday a Visual C# Forum Member posed an interesting question. The post can be found here : Possible BSTR memory leak

1.2 Basically, he designed a COM class (in IDL) that takes charge of acquiring data from some source. He also defined a callback interface that is to be implemented by clients. An instance of the COM class will invoke a method (named “AdviseCallback()”) of this callback interface when there is data to send.

1.3 His interest is with a client which is written in C#. The callback interface was implemented in C# :

[Guid("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"), InterfaceType(ComInterfaceType.InterfaceIsIDispatch)]
public interface IOPCCallback
{
    [DispId(1)]
    void AdviseCallback(string serverName, string channelName, string pointAddress, string aValue);

    [DispId(2)]
    void ReportError(string errorCode, string serverName, string channelName, string pointAddress);
}

[Guid("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")]
[ComVisible(true), ClassInterface(ClassInterfaceType.None)]
public class OPCCallback : IOPCCallback
{
    public OPCCallback()
    {
    }

    public void AdviseCallback(string serverName, string channelName, string pointAddress, string newValue)
    {
      //...rest of implementation
    }
    ...
    ...
    ...
}

1.4 When the AdviseCallback() event is fired, the data that is sent comes in the form of 4 BSTRs. The callback interface is IDispatch-based and the method is called via IDispatch::Invoke(). The following is a snippet of the code that he uses when his COM class fires the AdviseCallback() event :

DISPPARAMS dispparams;
DISPID dispid;
VARIANT VarResult = {0};

VARIANT arg[4];

CComBSTR value = szValue;  // szValue is LPCSTR
arg[0].vt = VT_BSTR;
arg[0].bstrVal = value.Detach();

CComBSTR itemName= szItemName;  // szItemName is LPCSTR
arg[1].vt = VT_BSTR;
arg[1].bstrVal = itemName.Detach();

CComBSTR groupName = szGroupName; // szGroupName is LPCSTR
arg[2].vt = VT_BSTR;
arg[2].bstrVal = groupName.Detach();

CComBSTR serverName = szServerName; // szServerName is LPCSTR
arg[3].vt = VT_BSTR;
arg[3].bstrVal = serverName.Detach();

dispparams.rgvarg = arg;
dispparams.cArgs = 4;
dispparams.cNamedArgs = 0;

LPOLESTR szMember = OLESTR("AdviseCallback");
hr = pOPCCallBack->GetIDsOfNames(IID_NULL, &szMember, 1, LOCALE_USER_DEFAULT,&dispid);

if (SUCCEEDED(hr))
{
 hr = pOPCCallBack->Invoke(dispid, IID_NULL,LOCALE_USER_DEFAULT, DISPATCH_METHOD, &dispparams, &VarResult,
  NULL, NULL);

 if (SUCCEEDED(hr))
 {
    //... and so on
 }
 ...
 ...
 ...

1.5 These are some of the pertinent points he raised about his call to the AdviseCallback() method via IDispatch :

  • He creates BSTRs via CComBSTR objects. The BSTR contained in each CComBSTR object is assigned to a VARIANT which is later passed to the IDispatch::Invoke() call as a method parameter.
  • The call to CComBSTR::Detach() sets to null its underlying BSTR pointer so that when the CComBSTR destructor calls ::SysFreeString(), no BSTR is actually freed because the internal BSTR pointer is NULL.
  • He asserted that this action is appropriate because it is the responsibility of the COM marshaler to release this memory.

1.6 The forum member then mentioned that through a gamut of test tools, he discovered memory leak connected with BSTRs. He suspected that the COM marshaler (i.e. the Type Library Marshaler) is not releasing memory when the client caller is a managed application, whereas it does when the client caller is native code based.

2. Analysis of the Issue.

2.1 Essentially, the forum member has misunderstood the basic concepts of memory ownership in relation to COM. This directly resulted in BSTR leakage.

2.2 The fact that the client was written in C# made no difference whatsoever. The forum member asserted that the BSTR leakage did not occur when the client was an unmanaged application but I begged to differ on this issue.

2.3 The sections that follow provide greater detail on the nature of the problem as well as a suggested solution.

3. Memory Ownership.

3.1 The following is a question from the forum member (partly edited by me for clarity) :

Note that the call to CComBSTR::Detach() sets to null the underlying BSTR pointer so that when the CComBSTR destructor invokes the underlying call to SysFreeString(), SysFreeString() will not release any memory since it is passed a null pointer. From what I have read, this is OK since in such cases it is the responsibility of the COM marshaller to release this memory (is this correct ?).

This is not correct. It is not necessarily the responsibility of the COM marshaler to release this memory. It all depends on how the AdviseCallback() method has been declared. From the code snippet listed in point 1.3 and 1.4, the AdviseCallback() method is likely declared as something like :

[id(1), helpstring("method AdviseCallback")] HRESULT AdviseCallback([in] BSTR ServerName, [in] BSTR GroupName, [in] BSTR ItemName, [in] BSTR Value);

3.2 Notice the [in] IDL attributes applied to the parameters. If this declaration is indeed the case, then the implementation of the method (the callee) must treat the incoming BSTRs as read-only. It certainly cannot free the BSTRs (via SysFreeString()).

3.3 The onus is on the caller of the method to allocate and then free the BSTRs. It is the caller that owns the memory of the BSTRs. The bug can be corrected by calling VariantClear() on each of the elements of the VARIANT array “arg”, after the call to Invoke(), e.g. :

 LPOLESTR szMember = OLESTR("AdviseCallback");
 hr = m_pIOPCCallBack->GetIDsOfNames(IID_NULL, &szMember, 1, LOCALE_USER_DEFAULT,&dispid);

 if (SUCCEEDED(hr))
 {
  hr = m_pIOPCCallBack->Invoke
       (
         dispid,
         IID_NULL,LOCALE_USER_DEFAULT,
         DISPATCH_METHOD,
         &dispparams,
         &VarResult,
         NULL,
         NULL
       );

  if (SUCCEEDED(hr))
  {
   //... and so on
  }
 }

 // Free the BSTRs contained inside the Variants.
 for (int i = 0; i < 4; i++)
 {
   VariantClear(&(arg[i]));
 }

3.4 Here is another question from the forum member :

It seems to me that given the food-chain I described above by which BSTR data is passed from the COM dll to the caller, it would be improper to release the BSTR memory in the COM dll since the associated BSTR pointers are passed to IDispatch::Invoke. This would mean that the releasing of the memory should either occur at the marshaler level or the client level, and that the COM dll code (shown below) is OK as is.

Negative. This is simply not the case. The direction of the parameters (i.e. the [in], [out] or [in, out] IDL attributes) determine this. In the case of AdviseCallback(), my strong guess (albeit I could be wrong) is that the 4 BSTR params are all [in]. Hence the BSTRs are to be allocated and freed by the caller and the callee must only read them (see also point 4.2 below).

3.5 For [out] parameters, it is the callee that allocates the memory to be returned to the caller. Once the caller receives this returned memory, it owns it and it is the caller that must free this memory.

3.6 For [in, out] parameters, it is the responsibility of the caller to allocate memory. However, the callee may choose to free it and then re-allocate new memory. The side that is eventually responsible for freeing the memory is still the caller.

4. Concerning the Memory Leakage.

4.1 The following are further questions from the forum member :

In the situation where the client is unmanaged, the memory leak did not show up. Only on the new platform (where the client is written in C#) does the memory leakage show up…

At first glance I would suspect that the COM marshaler is not releasing memory when the client caller is managed code, whereas it does when the client caller is native code. Could this be the case?

If the declaration of AdviseCallback() is indeed as listed in point 3.1 above, then I doubt very much that there is no BSTR leakage for unmanaged clients. There is most likely (undetected) memory leakage. The code listed in point 1.4 is definitely leaking BSTRs since it is the caller’s responsibility to free the BSTRs (and this is not happening).

4.2 Should the call to AdviseCallback() involve marshaling (e.g. across apartments/across managed and unmanaged code), then intermediate BSTRs may be created by the COM marshaler and/or the CLR’s Interop Marshaler. In these circumstances, the COM marshaler and/or the Interop Marshaler will free the intermediate BSTRs. However, the original BSTRs created in the calling code must be freed by the calling code.

4.3 There is only one very special situation in which memory is not leaked : the call to AdviseCallback() is done within the same apartment (i.e. no cross-apartment marshaling gets involved) and the code for AdviseCallback() (incorrectly) frees the BSTRs. However, this can only happen in unmanaged code.

4.4  This situation will work but it is not safe because, with the same code, if cross-apartment marshaling gets involved, then AdviseCallback() would have deleted the BSTRs that are meant to be deleted by the COM marshaler. A crash can follow.

5. BSTR Caching.

5.1 The forum member also asked the following question :

In pursuit of the above I also saw reverences to “turning off BSTR caching” (environment variable OANOCACHE = 1 to turn it off).  I assume that by default this caching is on.  Could this be the cause of leak in the managed code client scenario described here ? Would turning it off be proper to do?

5.2 BSTR caching is on by default. It is an optimization of the COM subsystem. It is generally a good feature in my opinion.

5.3 However, turning it off will not solve the problem if there is indeed BSTR leakage due to bugs (non-freeing of the BSTRs allocated in the calling code to AdviseCallback()).

5.4 However, it can be useful for development and testing purposes for checking that all relevant BSTRs are indeed freed.

Advertisements

About Lim Bio Liong

I've been in software development for nearly 20 years specializing in C , COM and C#. It's truly an exicting time we live in, with so much resources at our disposal to gain and share knowledge. I hope my blog will serve a small part in this global knowledge sharing network. For many years now I've been deeply involved with C development work. However since circa 2010, my current work has required me to use more and more on C# with a particular focus on COM interop. I've also written several articles for CodeProject. However, in recent years I've concentrated my time more on helping others in the MSDN forums. Please feel free to leave a comment whenever you have any constructive criticism over any of my blog posts.

Discussion

No comments yet.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: