Error in marshalling of unmanaged strings in .NET wrappers in RFC 68 implementation

[anilpatel]

There appears to be a bug in the marshalling of unmanaged ANSI strings across the unmanaged/managed boundary in the .NET wrappers due to changes made for RFC 68. This has caused heap corruption issues when the .NET wrappers are called on a product that uses the MapGuide OS code.

MapGuide RFC 68 can be found here:

​http://trac.osgeo.org/mapguide/wiki/MapGuideRfc68

The changelist to implement RFC 68 can be found here:

​http://trac.osgeo.org/mapguide/changeset/4006

The submission did not marshall ANSI strings correctly across the unmanaged/managed boundary. This resulted in heap corruption which was most readily evident in heap corruption assertions on 64-bit platforms, but should also be present in 32-bit platforms. The heap corruptions occurred at startup of the product using MapGuide OS code via the .NET API and would result in the product failing to successfully start.

In the submission, the following methods were added to MgObject:

virtual char* GetMultiByteClassName(); 
virtual char* GetNameSpace();

These methods returned a string with the class name or name space. On the unmanaged side, two new static global methods were added to the SWIG output in getclassid.code:

DllExport char* getClassName(void* ptrObj)
{
  return ((MgObject*)ptrObj)->GetMultiByteClassName();
}

DllExport char* getNameSpace(void* ptrObj)
{
  return ((MgObject*)ptrObj)->GetNameSpace();
}

These methods are exported from the unmanaged DLL and called from the managed wrapper DLL (also generated by SWIG) through the use of the DLLImport mechanism like this:

  [DllImport("unmanagedDllName", EntryPoint="getClassName")]
  public static extern string getClassName(IntPtr objectRef);

  [DllImport("unManagedDllName", EntryPoint="getNameSpace")]
  public static extern string getNameSpace(IntPtr objectRef);

The problem occurs because the DLLImport mechanism doesn't marshall the char * return value from getClassName/getNameSpace to the managed world. Marshalling of non-intrinsic return values like ANSI strings must be hanlded explicitly. SWIG does have a mechanism to do this for classes that it generates wrappers for itself. But the static global getClassName/getNameSpace methods are not part of the classes being wrapped and are instead being emitted literally in the SWIG changes.

To fix this issue, marshalling of the ANSI string via a shared memory buffer accessible via both managed and unmanaged code was implemented. The SWIGStringHelper (on the managed side) and SWIG_csharp_string_callback (on the unmanaged side) provided the prototype for the changes.

The new implementation of the static global methods on the unmanaged side call a managed code callback function to allocate shared memory. The ANSI string is copied into this memory and a pointer to the shared block is passed back via void/IntPtr to the managed wrapper. The code now looks like this:

DllExport void* getClassName(void* ptrObj)
{
    // Call virtual function
    void* jresult = 0;
    char* result = ((MgObject*)ptrObj)->GetMultiByteClassName();
    // Allocate common managed/unmanaged memory to hold string
    int buflength = (int)(strlen(result)+1)*sizeof(char *);
    jresult = SWIG_csharp_string_callback(buflength);
    // Copy string into common memory
    strncpy((char*)jresult, result, buflength);
    // Return ptr to common memory
    return jresult;
}

(similar changes to getNameSpace)

The return type should also be changed on the managed side DLLImports from string to IntPtr:

  [DllImport("unManagedDllName", EntryPoint="getClassName")]
  public static extern IntPtr getClassName(IntPtr objectRef);

  [DllImport("unManagedDllName", EntryPoint="getNameSpace")]
  public static extern IntPtr getNameSpace(IntPtr objectRef);

Since the getClassName and getNameSpace functions now return IntPtr, the createObject overload that took these two values as parameters needed to be modified to match the type. In addition, code was added to createObject to create a System.String from the ANSI string in the shared memory buffer and then release the shared memory buffer:

 static public object createObject(int id, IntPtr nameSpaceNamePtr, IntPtr 
classNamePtr, IntPtr cPtr, bool ownMemory)
  {
      // Marshall strings in nameSpaceNamePtr and classNamePtr from unmanaged 
char * to managed strings
      String nameSpaceName = 
System.Runtime.InteropServices.Marshal.PtrToStringAnsi(nameSpaceNamePtr);
      String className     = 
System.Runtime.InteropServices.Marshal.PtrToStringAnsi(classNamePtr);
      System.Runtime.InteropServices.Marshal.FreeCoTaskMem(nameSpaceNamePtr);
      System.Runtime.InteropServices.Marshal.FreeCoTaskMem(classNamePtr);

      ...

A patch file with the proposed fixes is attached to this ticket.

[tomfukushima]

Just reformatted a bit.

[anilpatel]

A slightly more efficient has been found and tested. This approach avoids creation/destruction of the shared memory area, thereby reducing the number of memory allocations/deallocations. The managed code accesses the returned string as an IntPtr directly and uses the .NET InteropServices to marshal the unmanaged Ansi string to a managed string.

New approach:

Changes required to unmanaged side: NONE. Code remains as it is in the vault/depot:

DllExport char* getClassName(void* ptrObj)
{
  return ((MgObject*)ptrObj)->GetMultiByteClassName();
}

DllExport char* getNameSpace(void* ptrObj)
{
  return ((MgObject*)ptrObj)->GetNameSpace();
}

Changes required to the managed side DllImports (same as original proposal):

  [DllImport("unManagedDllName", EntryPoint="getClassName")]
  public static extern IntPtr getClassName(IntPtr objectRef);

  [DllImport("unManagedDllName", EntryPoint="getNameSpace")]
  public static extern IntPtr getNameSpace(IntPtr objectRef);

Changes required to createObject overload:

 static public object createObject(int id, IntPtr nameSpaceNamePtr, IntPtr 
classNamePtr, IntPtr cPtr, bool ownMemory)
  {
      // Marshall strings in nameSpaceNamePtr and classNamePtr from unmanaged 
char * to managed strings
      String nameSpaceName = 
System.Runtime.InteropServices.Marshal.PtrToStringAnsi(nameSpaceNamePtr);
      String className     = 
System.Runtime.InteropServices.Marshal.PtrToStringAnsi(classNamePtr);

      ...

The attached patch file has been updated to reflect the new proposal.

[anilpatel]

Proposed fixes to .NET wrappers

[tomfukushima]

Submitted by Leaf: r4091. Closing ticket.