Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal of COMThread broke simplification usecase of the COM.util package #651

Closed
matthiasblaesing opened this issue May 2, 2016 · 6 comments

Comments

@matthiasblaesing
Copy link
Member

@dhakehurst raised in a comment to pullrequest #649 the following issue in regard to the removal of the COMThread from jna codebase:

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.

matthiasblaesing referenced this issue May 2, 2016
…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.
@matthiasblaesing
Copy link
Member Author

Thanks for starting this discussion - indeed I failed to explain why I went ahead and removed the COMThread. While it looked like a good idea when looking at it at first, I became less enthusiastic at that time.

Basicly the util classes have a different approach to the basic COM classes, that uses much less boiler plate code and move the common code into the basic module (wrapping, dispatch handling, memory handling) and this can now used at the price, that you need to initialize COM in the threads that access the COM methods.

While I see that this is break in the API (a massive one), I consider it worth it. The proxy approach can now be used in MTA and STA mode, provided the user prepares the environment correctly (that would be calling the CoInitializeEx/CoUninitialize pair).

In fact the save part of the current api could be reimplemented see:

https://gist.github.com/matthiasblaesing/72c0b4751d52ebfd4406a72e5c7fe6d0

The idea, layer a single threaded factory on top of the basic factory. This factory wraps the COM objects into a layer, that pushes them into the thread executor.

I still would strongly advice against using callbacks in this scenario, but for a basic usecase this might be ok.

At first I tried to stay with the single thread approach, but then I tried to enhance it and uglyness entered. To be able to handle callbacks, I introduces a "COM initialized" flag, that would allow the callback and the factory to be used in the callback thread (which is by definition COM initialized), but that flag needed to be maintained.

Another pain point were the places, where calls had to be pushed into the COM thread. Then Interrupted-, Execution- and TimeoutException were caught and rethrown as a RuntimeException. The interesting question: what happens when the COM call comes back, after the TimeoutException happened? In all fairness I have no idea, but I don't want to try.

@dhakehurst
Copy link

My usecase for this involves many different java threads accessing the COM objects, hence the reason I included the ComThread in my original implementation of this.

I accept that this does make it a bottle neck, and there may be other usecases that require something like you suggest.

Is there a way that we can have both options available?

@matthiasblaesing
Copy link
Member Author

@dhakehurst did you have a look at the gist I referenced? The idea is, that you can replace the Factory class with COMFactory, that will do the same "move calling into one central thread" handling, that the old Factory did.

I ran the Word MSDN sample through this and that works as I'd expect. I have to think about the callback handlers, but maybe I could retrofit the "mark thread as COM thread approach".

If this works out, I see this option:

  • move the current Factory to another name - "BaseFactory" or something along the lines
  • reintroduce COMThread
  • Move COMFactory from the gist into the project under the name "Factory"

@dhakehurst
Copy link

That sounds like it could work.
I can't look in detail until next week.
But I will do.

On Thursday, 5 May 2016, matthiasblaesing [email protected] wrote:

@dhakehurst https://github.com/dhakehurst did you have a look at the
gist I referenced? The idea is, that you can replace the Factory class with
COMFactory, that will do the same "move calling into one central thread"
handling, that the old Factory did.

I ran the Word MSDN sample through this and that works as I'd expect. I
have to think about the callback handlers, but maybe I could retrofit the
"mark thread as COM thread approach".

If this works out, I see this option:

  • move the current Factory to another name - "BaseFactory" or
    something along the lines
  • reintroduce COMThread
  • Move COMFactory from the gist into the project under the name
    "Factory"


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#651 (comment)

@matthiasblaesing
Copy link
Member Author

@dhakehurst - I made a prototype implementation available as:

https://github.com/matthiasblaesing/jna/tree/simple_com_htread

I added unittests to cover the new baseclass ObjectFactory and the old Factory. This is pretty much the implementation sketched above.

@dhakehurst
Copy link

this looks like it could work,
I will need a bit more time to test it out.

On 8 May 2016 at 16:56, matthiasblaesing [email protected] wrote:

@dhakehurst https://github.com/dhakehurst - I made a prototype
implementation available as:

https://github.com/matthiasblaesing/jna/tree/simple_com_htread

I added unittests to cover the new baseclass ObjectFactory and the old
Factory. This is pretty much the implementation sketched above.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#651 (comment)

mstyura pushed a commit to mstyura/jna that referenced this issue Sep 9, 2024
Motivation:

Quiche did a new release, update the sha to it.

Modifications:

Update to sha which is quiche 0.3.0

Result:

Use latest "official" release of quiche
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants