Correct freeing of BSTRs

According to MSDN the caller is responsible to free BSTRs.
java-native-access · Feb 21, 2016 · 78e6d0f · 78e6d0f
1 parent 334ae3f
commit 78e6d0f
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 31 deletions.
diff --git a/contrib/platform/src/com/sun/jna/platform/win32/COM/util/Convert.java b/contrib/platform/src/com/sun/jna/platform/win32/COM/util/Convert.java
@@ -12,6 +12,8 @@
  */
 package com.sun.jna.platform.win32.COM.util;
 
+import com.sun.jna.platform.win32.OleAuto;
+import com.sun.jna.platform.win32.Variant;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -21,9 +23,29 @@
 import com.sun.jna.platform.win32.WTypes;
 import com.sun.jna.platform.win32.WinDef;
 import com.sun.jna.platform.win32.Variant.VARIANT;
+import com.sun.jna.platform.win32.WTypes.BSTR;
 
-public class Convert {
-
+/**
+ * This class is considered internal to the package.
+ */
+class Convert {
+ /**
+ * Convert a java value into a VARIANT suitable for passing in a COM
+ * invocation.
+ * 
+ * <p><i>Implementation notes</i></p>
+ * 
+ * <ul>
+ * <li>VARIANTs are not rewrapped, but passed through unmodified</li>
+ * <li>A string is wrapped into a BSTR, that is wrapped into the VARIANT.
+ * The string is allocated as native memory by the VARIANT constructor.
+ * The BSTR needs to be freed by {@see com.sun.jna.platform.win32.OleAuto#SysFreeString}.</li>
+ * </ul>
+ * 
+ * @see com.sun.jna.platform.win32.Variant.VARIANT#VARIANT(java.lang.String)
+ * @param value to be wrapped
+ * @return wrapped VARIANT
+ */
  public static VARIANT toVariant(Object value) {
  if (value instanceof VARIANT) {
  return (VARIANT) value;
@@ -108,4 +130,39 @@ public static <T extends IComEnum> T toComEnum(Class<T> enumType, Object value)
  }
  return null;
  }
+
+ /**
+ * Free the contents of the supplied VARIANT.
+ * 
+ * <p>This method is a companion to {@see #toVariant}. Primary usage is
+ * to free BSTRs contained in VARIANTs.</p>
+ * 
+ * @param variant to be cleared
+ * @param javaType type before/after conversion
+ */
+ public static void free(VARIANT variant, Class<?> javaType) {
+ if(javaType == null) {
+ return;
+ }
+ if(javaType.isAssignableFrom(String.class) 
+ && variant.getVarType().intValue() == Variant.VT_BSTR) {
+ Object value = variant.getValue();
+ if(value instanceof BSTR) {
+ OleAuto.INSTANCE.SysFreeString((BSTR) value);
+ }
+ }
+ }
+
+ /**
+ * Free the contents of the supplied VARIANT.
+ * 
+ * <p>This method is a companion to {@see #toVariant}. Primary usage is
+ * to free BSTRs contained in VARIANTs.</p>
+ * 
+ * @param variant to be cleared
+ * @param value value before/after conversion
+ */
+ public static void free(VARIANT variant, Object value) {
+ free(variant, value == null ? null : value.getClass());
+ }
 }
diff --git a/contrib/platform/src/com/sun/jna/platform/win32/COM/util/ProxyObject.java b/contrib/platform/src/com/sun/jna/platform/win32/COM/util/ProxyObject.java
@@ -354,9 +354,10 @@ public HRESULT call() throws Exception {
  // --------------------- IDispatch ------------------------------
  @Override
  public <T> void setProperty(String name, T value) {
- VARIANT v = Convert.toVariant(value);
- WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_PROPERTYPUT, null, this.getRawDispatch(), name, v);
- COMUtils.checkRC(hr);
+ VARIANT v = Convert.toVariant(value);
+ WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_PROPERTYPUT, null, this.getRawDispatch(), name, v);
+ Convert.free(v, value); // Free value allocated by Convert#toVariant
+ COMUtils.checkRC(hr);
  }
 
  @Override
@@ -372,20 +373,15 @@ public <T> T getProperty(Class<T> returnType, String name, Object... args) {
  }
  Variant.VARIANT.ByReference result = new Variant.VARIANT.ByReference();
  WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_PROPERTYGET, result, this.getRawDispatch(), name, vargs);
+
+ for (int i = 0; i < vargs.length; i++) {
+ // Free value allocated by Convert#toVariant
+ Convert.free(vargs[i], args[i]);
+ }
+
  COMUtils.checkRC(hr);
- Object jobj = Convert.toJavaObject(result, returnType);
- if (IComEnum.class.isAssignableFrom(returnType)) {
- return returnType.cast(Convert.toComEnum((Class<? extends IComEnum>) returnType, jobj));
- }
- if (jobj instanceof IDispatch) {
- IDispatch d = (IDispatch) jobj;
- T t = this.factory.createProxy(returnType, d);
- // must release a COM reference, createProxy adds one, as does the
- // call
- int n = d.Release();
- return t;
- }
- return returnType.cast(jobj);
+
+ return convertAndFreeReturn(result, returnType);
  }
 
  @Override
@@ -401,23 +397,34 @@ public <T> T invokeMethod(Class<T> returnType, String name, Object... args) {
  }
  Variant.VARIANT.ByReference result = new Variant.VARIANT.ByReference();
  WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_METHOD, result, this.getRawDispatch(), name, vargs);
+
+ for (int i = 0; i < vargs.length; i++) {
+ // Free value allocated by Convert#toVariant
+ Convert.free(vargs[i], args[i]);
+ }
+
  COMUtils.checkRC(hr);
 
- Object jobj = Convert.toJavaObject(result, returnType);
- if (IComEnum.class.isAssignableFrom(returnType)) {
- return returnType.cast(Convert.toComEnum((Class<? extends IComEnum>) returnType, jobj));
- }
- if (jobj instanceof IDispatch) {
- IDispatch d = (IDispatch) jobj;
- T t = this.factory.createProxy(returnType, d);
- // must release a COM reference, createProxy adds one, as does the
- // call
- int n = d.Release();
- return t;
- }
- return returnType.cast(jobj);
+ return convertAndFreeReturn(result, returnType);
  }
 
+ private <T> T convertAndFreeReturn(VARIANT.ByReference result, Class<T> returnType) {
+ Object jobj = Convert.toJavaObject(result, returnType);
+ if (IComEnum.class.isAssignableFrom(returnType)) {
+ return returnType.cast(Convert.toComEnum((Class<? extends IComEnum>) returnType, jobj));
+ } else if (jobj instanceof IDispatch) {
+ IDispatch d = (IDispatch) jobj;
+ T t = this.factory.createProxy(returnType, d);
+ // must release a COM reference, createProxy adds one, as does the
+ // call
+ int n = d.Release();
+ return t;
+ } else {
+ Convert.free(result, returnType);
+ return returnType.cast(jobj);
+ }
+ }
+
  @Override
  public <T> T queryInterface(Class<T> comInterface) throws COMException {
  try {

diff --git a/contrib/platform/src/com/sun/jna/platform/win32/Variant.java b/contrib/platform/src/com/sun/jna/platform/win32/Variant.java
@@ -234,6 +234,16 @@ public VARIANT(double value) {
  this.setValue(VT_R8, value);
  }
 
+ /**
+ * Create a new VARIANT wrapping the supplied string.
+ * 
+ * <p><i>Implementation note:</i> the string is wrapped as a BSTR value,
+ * that is allocated using {@see com.sun.jna.platform.win32.OleAuto#SysAllocString}
+ * and needs to be freed using
+ * {@see com.sun.jna.platform.win32.OleAuto#SysFreeString} by the user</p>
+ * 
+ * @param value to be wrapped
+ */
  public VARIANT(String value) {
  this();
  BSTR bstrValue = OleAuto.INSTANCE.SysAllocString(value);