Skip to content

Commit

Permalink
Remove COM initialization from object and enforce correct handling vi…
Browse files Browse the repository at this point in the history 
…a asserts

The custom thread instantiated in the c.s.j.p.w.C.util.ProxyObject and
then used as a dispatch for COM calls is a bootleneck and not following
COM convention.

If COM is initialized for a thread as COINIT_MULTITHREADED there is no
reason to force dispatch through a central thread and if it is 
COINIT_APARTMENTTHREADED a message queue is needed:

https://msdn.microsoft.com/de-de/library/windows/desktop/ms695279%28v=vs.85%29.aspx

The change introduced here removes the dispatcher thread and the
initialization routines from COMBindingBaseObject. In their place asserts
are placed, that enforce correct COM initialization. The tests were
adjusted accordingly.
  • Loading branch information
@matthiasblaesing
matthiasblaesing committed Feb 28, 2016
1 parent bf32054 commit d80730b
Show file tree
Hide file tree
Showing 24 changed files with 349 additions and 567 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Bug Fixes
* [#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)
* [#601](https://github.com/java-native-access/jna/pull/601): Remove COMThread and COM initialization from objects and require callers to initialize COM themselves. Asserts are added to guard correct usage. - [@matthiasblaesing](https://github.com/matthiasblaesing).

Release 4.2.1
=============
Expand Down
29 changes: 9 additions & 20 deletions contrib/platform/src/com/sun/jna/platform/win32/COM/COMBindingBaseObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.sun.jna.ptr.IntByReference;
import com.sun.jna.ptr.PointerByReference;

// TODO: Auto-generated Javadoc
/**
* Helper class to provide basic COM support.
*
Expand Down Expand Up @@ -73,14 +72,10 @@ public COMBindingBaseObject(CLSID clsid, boolean useActiveInstance) {

public COMBindingBaseObject(CLSID clsid, boolean useActiveInstance,
int dwClsContext) {
// Initialize COM for this thread...
HRESULT hr = Ole32.INSTANCE.CoInitializeEx(null, Ole32.COINIT_APARTMENTTHREADED);

if (COMUtils.FAILED(hr)) {
Ole32.INSTANCE.CoUninitialize();
throw new COMException("CoInitialize() failed!");
}

assert COMUtils.comIsInitialized() : "COM not initialized";

HRESULT hr;

if (useActiveInstance) {
hr = OleAuto.INSTANCE.GetActiveObject(clsid, null, this.pUnknown);

Expand All @@ -107,20 +102,15 @@ public COMBindingBaseObject(CLSID clsid, boolean useActiveInstance,

public COMBindingBaseObject(String progId, boolean useActiveInstance,
int dwClsContext) throws COMException {
// Initialize COM for this thread...
HRESULT hr = Ole32.INSTANCE.CoInitializeEx(null, Ole32.COINIT_APARTMENTTHREADED);

if (COMUtils.FAILED(hr)) {
this.release();
throw new COMException("CoInitialize() failed!");
}
assert COMUtils.comIsInitialized() : "COM not initialized";

HRESULT hr;

// Get CLSID for Word.Application...
CLSID.ByReference clsid = new CLSID.ByReference();
hr = Ole32.INSTANCE.CLSIDFromProgID(progId, clsid);

if (COMUtils.FAILED(hr)) {
Ole32.INSTANCE.CoUninitialize();
throw new COMException("CLSIDFromProgID() failed!");
}

Expand Down Expand Up @@ -194,10 +184,9 @@ public PointerByReference getIUnknownPointer() {
* Release.
*/
public void release() {
if (this.iDispatch != null)
if (this.iDispatch != null) {
this.iDispatch.Release();

Ole32.INSTANCE.CoUninitialize();
}
}

protected HRESULT oleMethod(int nType, VARIANT.ByReference pvResult,
Expand Down
34 changes: 33 additions & 1 deletion contrib/platform/src/com/sun/jna/platform/win32/COM/COMUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,21 @@
import java.util.ArrayList;

import com.sun.jna.Native;
import com.sun.jna.Pointer;
import com.sun.jna.platform.win32.Advapi32;
import com.sun.jna.platform.win32.Advapi32Util;
import com.sun.jna.platform.win32.Advapi32Util.EnumKey;
import com.sun.jna.platform.win32.Advapi32Util.InfoKey;
import com.sun.jna.platform.win32.Kernel32Util;
import com.sun.jna.platform.win32.OaIdl.EXCEPINFO;
import com.sun.jna.platform.win32.Ole32;
import com.sun.jna.platform.win32.W32Errors;
import com.sun.jna.platform.win32.WinNT;
import com.sun.jna.platform.win32.WinNT.HRESULT;
import com.sun.jna.platform.win32.WinReg;
import com.sun.jna.platform.win32.WinReg.HKEYByReference;
import com.sun.jna.ptr.IntByReference;

// TODO: Auto-generated Javadoc
/**
* The Class COMUtils.
*
Expand Down Expand Up @@ -182,6 +184,36 @@ public static ArrayList<COMInfo> getAllCOMInfoOnSystem() {
return comInfos;
}

/**
* Check is COM was initialized correctly. The initialization status is not changed!
*
* <p>This is a debug function, not for normal usage!</p>
*
* @return
*/
public static boolean comIsInitialized() {
WinNT.HRESULT hr = Ole32.INSTANCE.CoInitializeEx(Pointer.NULL, Ole32.COINIT_MULTITHREADED);
if (hr.equals(W32Errors.S_OK)) {
// User failed - uninitialize again and return false
Ole32.INSTANCE.CoUninitialize();
return false;
} else if (hr.equals(W32Errors.S_FALSE)) {
// OK Variant 1 - User initialized COM with same threading module as
// in this check. According to MSDN CoUninitialize needs to be called
// in this case.
Ole32.INSTANCE.CoUninitialize();
return true;
} else if (hr.intValue() == W32Errors.RPC_E_CHANGED_MODE) {
return true;
}
// If another result than the checked ones above happens handling is
// delegated to the "normal" COM exception handling and a COMException
// will be raised.
COMUtils.checkRC(hr);
// The return will not be met, as COMUtils#checkRC will raise an exception
return false;
}

/**
* The Class COMInfo.
*
Expand Down
13 changes: 5 additions & 8 deletions contrib/platform/src/com/sun/jna/platform/win32/COM/util/CallbackProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.sun.jna.platform.win32.WinError;
import com.sun.jna.platform.win32.WinNT.HRESULT;
import com.sun.jna.platform.win32.COM.COMException;
import com.sun.jna.platform.win32.COM.COMUtils;
import com.sun.jna.platform.win32.COM.Dispatch;
import com.sun.jna.platform.win32.COM.DispatchListener;
import com.sun.jna.platform.win32.COM.IDispatch;
Expand Down Expand Up @@ -103,7 +104,7 @@ int fetchDispIdFromName(ComEventCallback annotation) {

void invokeOnThread(final DISPID dispIdMember, final REFIID riid, LCID lcid, WORD wFlags,
final DISPPARAMS.ByReference pDispParams) {

final Method eventMethod;
if (CallbackProxy.this.dsipIdMap.containsKey(dispIdMember)) {
eventMethod = CallbackProxy.this.dsipIdMap.get(dispIdMember);
Expand Down Expand Up @@ -204,13 +205,9 @@ public HRESULT Invoke(DISPID dispIdMember, REFIID riid, LCID lcid, WORD wFlags,
DISPPARAMS.ByReference pDispParams, VARIANT.ByReference pVarResult, EXCEPINFO.ByReference pExcepInfo,
IntByReference puArgErr) {

assert (! ComThread.getCurrentThreadIsCOM()) : "Assumption about COM threading broken.";
ComThread.setCurrentThreadIsCOM(true);
try {
this.invokeOnThread(dispIdMember, riid, lcid, wFlags, pDispParams);
} finally {
ComThread.setCurrentThreadIsCOM(false);
}
assert COMUtils.comIsInitialized() : "Assumption about COM threading broken.";

this.invokeOnThread(dispIdMember, riid, lcid, wFlags, pDispParams);

return WinError.S_OK;
}
Expand Down
157 changes: 0 additions & 157 deletions contrib/platform/src/com/sun/jna/platform/win32/COM/util/ComThread.java
View file

This file was deleted.

Loading

4 comments on commit d80730b

@dhakehurst
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the purpose of the ...COM.util classes (ProxyObject etc) is (was) to create a 'nicer' java like interface over the COM interactions.
For example, so that the user of the classes does not need to know anything about COM threading issues,
and doesn't have to make calls such as "Ole32.INSTANCE.CoInitializeEx(Pointer.NULL, Ole32.COINIT_MULTITHREADED);"

If the client codes wants to have that level of control it should use the ...COM classes directly.

Your recent modifications of the COM.util classes have changed the use case significantly,
and consequently I cannot move my client code to using the latest version of JNA.

Would you be happy for a discussion about this?

thanks

@matthiasblaesing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, my email is part of the commits, so feel free to write there or open a bug, I'll follow up there. Or use the google group.

@dblock
Copy link
Member

@dblock dblock commented on d80730b May 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep this all public, open issues please and discuss there.

@matthiasblaesing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the request into issue #651.

Please sign in to comment.