Fix some of the failing COM unittests

CHANGES.md

@@ -48,6 +48,7 @@ Bug Fixes
 * [#584](https://github.com/java-native-access/jna/pull/584): Promote float varargs to double - [@marco2357](https://github.com/marco2357).
 * [#588](https://github.com/java-native-access/jna/pull/588): Fix varargs calls on arm - [@twall](https://github.com/twall).
 * [#593](https://github.com/java-native-access/jna/pull/593): Improve binding of TypeLib bindings - [@matthiasblaesing](https://github.com/matthiasblaesing).
+* [#578](https://github.com/java-native-access/jna/pull/578): Fix COM CallbackHandlers, allow usage of VARIANTs directly in c.s.j.p.w.COM.util.ProxyObject and fix native memory leak in c.s.j.p.w.COM.util.ProxyObject - [@matthiasblaesing](https://github.com/matthiasblaesing)
 
 Release 4.2.1
 =============

contrib/platform/src/com/sun/jna/platform/win32/COM/util/CallbackProxy.java

@@ -12,15 +12,11 @@
  */
 package com.sun.jna.platform.win32.COM.util;
 
-import java.lang.Thread.UncaughtExceptionHandler;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
 
 import com.sun.jna.Pointer;
 import com.sun.jna.WString;
@@ -61,20 +57,6 @@ public CallbackProxy(Factory factory, Class<?> comEventCallbackInterface,
  this.listenedToRiid = this.createRIID(comEventCallbackInterface);
  this.dsipIdMap = this.createDispIdMap(comEventCallbackInterface);
  this.dispatchListener = new DispatchListener(this);
- this.executorService = Executors.newSingleThreadExecutor(new ThreadFactory() {
- @Override
- public Thread newThread(Runnable r) {
- Thread thread = new Thread(r, "COM Event Callback executor");
- thread.setDaemon(true);
- thread.setUncaughtExceptionHandler(new UncaughtExceptionHandler() {
- @Override
- public void uncaughtException(Thread t, Throwable e) {
- CallbackProxy.this.factory.comThread.uncaughtExceptionHandler.uncaughtException(t, e);
- }
- });
- return thread;
- }
- });
  }
 
  Factory factory;
@@ -83,7 +65,6 @@ public void uncaughtException(Thread t, Throwable e) {
  REFIID listenedToRiid;
  public DispatchListener dispatchListener;
  Map<DISPID, Method> dsipIdMap;
- ExecutorService executorService;
 
  REFIID createRIID(Class<?> comEventCallbackInterface) {
  ComInterface comInterfaceAnnotation = comEventCallbackInterface.getAnnotation(ComInterface.class);
@@ -121,84 +102,80 @@ int fetchDispIdFromName(ComEventCallback annotation) {
  }
 
  void invokeOnThread(final DISPID dispIdMember, final REFIID riid, LCID lcid, WORD wFlags,
- final DISPPARAMS.ByReference pDispParams) {
- // decode arguments
- // must decode them on this thread, and create a proxy for any COM objects (IDispatch)
- // this will AddRef on the COM object so that it is not cleaned up before we can use it
- // on the thread that does the java callback.
- List<Object> rjargs = new ArrayList<Object>();
- if (pDispParams.cArgs.intValue() > 0) {
- VariantArg vargs = pDispParams.rgvarg;
- vargs.setArraySize(pDispParams.cArgs.intValue());
- for (Variant.VARIANT varg : vargs.variantArg) {
- Object jarg = Convert.toJavaObject(varg);
- if (jarg instanceof IDispatch) {
- IDispatch dispatch = (IDispatch) jarg;
- //get raw IUnknown interface
- PointerByReference ppvObject = new PointerByReference();
- IID iid = com.sun.jna.platform.win32.COM.IUnknown.IID_IUNKNOWN;
- dispatch.QueryInterface(new REFIID(iid), ppvObject);
- Unknown rawUnk = new Unknown(ppvObject.getValue());
+ final DISPPARAMS.ByReference pDispParams) {
+
+ final Method eventMethod;
+ if (CallbackProxy.this.dsipIdMap.containsKey(dispIdMember)) {
+ eventMethod = CallbackProxy.this.dsipIdMap.get(dispIdMember);
+ if (eventMethod.getParameterTypes().length != pDispParams.cArgs.intValue()) {
+ CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
+ "Trying to invoke method " + eventMethod + " with " + pDispParams.cArgs.intValue() + " arguments",
+ null);
+ return;
+ }
+ } else {
+ CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
+ "No method found with dispId = " + dispIdMember, null);
+ return;
+ }
+
+ // Arguments are converted to the JAVA side and IDispatch Interfaces
+ // are wrapped into an ProxyObject if so requested.
+ //
+ // Out-Parameter need to be specified as VARIANT, VARIANT args are
+ // not converted, so COM memory allocation rules apply.
+ final Class<?>[] params = eventMethod.getParameterTypes();
+ List<Object> rjargs = new ArrayList<Object>();
+ if (pDispParams.cArgs.intValue() > 0) {
+ VariantArg vargs = pDispParams.rgvarg;
+ vargs.setArraySize(pDispParams.cArgs.intValue());
+ for ( int i = 0; i < vargs.variantArg.length; i++) {
+ Class targetClass = params[vargs.variantArg.length - 1 - i];
+ Variant.VARIANT varg = vargs.variantArg[i];
+ Object jarg = Convert.toJavaObject(varg, targetClass);
+ if (jarg instanceof IDispatch) {
+ // If a dispatch is returned try to wrap it into a proxy 
+ // helper if the target is ComInterface annotated
+ IDispatch dispatch = (IDispatch) jarg;
+ //get raw IUnknown interface
+ PointerByReference ppvObject = new PointerByReference();
+ IID iid = com.sun.jna.platform.win32.COM.IUnknown.IID_IUNKNOWN;
+ dispatch.QueryInterface(new REFIID(iid), ppvObject);
+ Unknown rawUnk = new Unknown(ppvObject.getValue());
  long unknownId = Pointer.nativeValue( rawUnk.getPointer() );
- int n = rawUnk.Release();
- //Note: unlike in other places, there is currently no COM ref already added for this pointer 
- IUnknown unk = CallbackProxy.this.factory.createProxy(IUnknown.class, unknownId, dispatch);
- rjargs.add(unk);
- } else {
- rjargs.add(jarg);
- }
- }
- }
- final List<Object> jargs = new ArrayList<Object>(rjargs);
- Runnable invokation = new Runnable() {
- @Override
- public void run() {
- try {
- if (CallbackProxy.this.dsipIdMap.containsKey(dispIdMember)) {
- Method eventMethod = CallbackProxy.this.dsipIdMap.get(dispIdMember);
- if (eventMethod.getParameterTypes().length != jargs.size()) {
- CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
- "Trying to invoke method " + eventMethod + " with " + jargs.size() + " arguments",
- null);
- } else {
- try {
- // need to convert arguments maybe
- List<Object> margs = new ArrayList<Object>();
- Class<?>[] params = eventMethod.getParameterTypes();
- for (int i = 0; i < eventMethod.getParameterTypes().length; ++i) {
- Class<?> paramType = params[i];
- Object jobj = jargs.get(i);
- if (jobj != null && paramType.getAnnotation(ComInterface.class) != null) {
- if (jobj instanceof IUnknown) {
- IUnknown unk = (IUnknown) jobj;
- Object mobj = unk.queryInterface(paramType);
- margs.add(mobj);
- } else {
- throw new RuntimeException("Cannot convert argument " + jobj.getClass()
- + " to ComInterface " + paramType);
- }
- } else {
- margs.add(jobj);
- }
- }
- eventMethod.invoke(comEventCallbackListener, margs.toArray());
- } catch (Exception e) {
- CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
- "Exception invoking method " + eventMethod, e);
- }
- }
- } else {
- CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
- "No method found with dispId = " + dispIdMember, null);
- }
- } catch (Exception e) {
- CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
- "Exception receiving callback event ", e);
- }
- }
- };
- this.executorService.execute(invokation);
- }
+ IUnknown unk = CallbackProxy.this.factory.createProxy(IUnknown.class, unknownId, dispatch);
+ if(targetClass.getAnnotation(ComInterface.class) != null) {
+ rjargs.add(unk.queryInterface(targetClass));
+ } else {
+ rjargs.add(unk);
+ }
+ } else {
+ rjargs.add(jarg);
+ }
+ }
+ }
+
+ List<Object> margs = new ArrayList<Object>();
+ try {
+ // Reverse order from calling convention
+ int lastParamIdx = eventMethod.getParameterTypes().length - 1;
+ for (int i = lastParamIdx; i >= 0; i--) {
+ margs.add(rjargs.get(i));
+ }
+ eventMethod.invoke(comEventCallbackListener, margs.toArray());
+ } catch (Exception e) {
+ List<String> decodedClassNames = new ArrayList<String>(margs.size());
+ for(Object o: margs) {
+ if(o == null) {
+ decodedClassNames.add("NULL");
+ } else {
+ decodedClassNames.add(o.getClass().getName());
+ }
+ }
+ CallbackProxy.this.comEventCallbackListener.errorReceivingCallbackEvent(
+ "Exception invoking method " + eventMethod + " supplied: " + decodedClassNames.toString(), e);
+ }
+ }
 
  @Override
  public Pointer getPointer() {
@@ -227,7 +204,13 @@ public HRESULT Invoke(DISPID dispIdMember, REFIID riid, LCID lcid, WORD wFlags,
  DISPPARAMS.ByReference pDispParams, VARIANT.ByReference pVarResult, EXCEPINFO.ByReference pExcepInfo,
  IntByReference puArgErr) {
 
- this.invokeOnThread(dispIdMember, riid, lcid, wFlags, pDispParams);
+ assert (! ComThread.getCurrentThreadIsCOM()) : "Assumption about COM threading broken.";
+ ComThread.setCurrentThreadIsCOM(true);
+ try {
+ this.invokeOnThread(dispIdMember, riid, lcid, wFlags, pDispParams);
+ } finally {
+ ComThread.setCurrentThreadIsCOM(false);
+ }
 
  return WinError.S_OK;
  }

contrib/platform/src/com/sun/jna/platform/win32/COM/util/ComThread.java

@@ -26,7 +26,6 @@
 import com.sun.jna.platform.win32.COM.COMUtils;
 
 public class ComThread {
-
  ExecutorService executor;
  Runnable firstTask;
  boolean requiresInitialisation;
@@ -44,6 +43,8 @@ public ComThread(final String threadName, long timeoutMilliseconds, UncaughtExce
  this.firstTask = new Runnable() {
  @Override
  public void run() {
+ // By definition this is a COM thread
+ ComThread.setCurrentThreadIsCOM(true);
  try {
  //If we do not use COINIT_MULTITHREADED, it is necessary to have
  // a message loop see -
@@ -119,11 +120,38 @@ protected void finalize() throws Throwable {
  }
  }
 
+ // The currentThreadIsCOM is used if wrapper are used in a callback
+ // the callback is called in a new thread by the JNA runtime. As the
+ // call comes from COM it is asumed, that the thread is correctly
+ // initialized and can be used for COM calls (see MTA assumption above)
+ private static ThreadLocal<Boolean> currentThreadIsCOM = new ThreadLocal<Boolean>();
+
+ static void setCurrentThreadIsCOM(boolean isCOM) {
+ currentThreadIsCOM.set(isCOM);
+ }
+
+ static boolean getCurrentThreadIsCOM() {
+ Boolean res = currentThreadIsCOM.get();
+ if(res == null) {
+ return false;
+ } else {
+ return currentThreadIsCOM.get();
+ }
+ }
+
  public <T> T execute(Callable<T> task) throws TimeoutException, InterruptedException, ExecutionException {
- if (this.requiresInitialisation) {
- executor.execute(firstTask);
- }
- return executor.submit(task).get(this.timeoutMilliseconds, TimeUnit.MILLISECONDS);
+ if(getCurrentThreadIsCOM()) {
+ try {
+ return task.call();
+ } catch (Exception ex) {
+ throw new ExecutionException(ex);
+ }
+ } else {
+ if (this.requiresInitialisation) {
+ executor.execute(firstTask);
+ }
+ return executor.submit(task).get(this.timeoutMilliseconds, TimeUnit.MILLISECONDS);
+ }
  }
 
 }