Crash caused by incorrectly generated code for VARIANT function input

[schernikov]

While working with AutoCAD objects ran into app crash. It was reported sometimes as stack overflow or access violation. Investigation showed that there was a method signature generated as:

IAcadBlock._methods_ = [
    COMMETHOD([dispid(0), helpstring(u'Gets the member object at a given index in a collection, group, or selection set')], HRESULT, 'Item',
              ( ['in'], VARIANT, 'Index' ),
              ( ['retval', 'out'], POINTER(POINTER(IAcadEntity)), 'pItem' )),

Manual tweaking and change to (Note POINTER(VARIANT) vs VARIANT as an input):

IAcadBlock._methods_ = [
    COMMETHOD([dispid(0), helpstring(u'Gets the member object at a given index in a collection, group, or selection set')], HRESULT, 'Item',
              ( ['in'], POINTER(VARIANT), 'Index' ),
              ( ['retval', 'out'], POINTER(POINTER(IAcadEntity)), 'pItem' )),

made it run as expected.

Not sure yet where this has to be fixed to make it permanent in the code generation logic.

I can provide tlb file used to produce generated code. Please let me know.

[cfarrow]

Thanks for the bug report! We willl need the tlb before starting on this. Someone will be in touch when we start on this.

[schernikov]

Here is the tlb file and python file generated from it.

Please look at line #9962 in _E072BCE4_9027_4F86_BAE2_EF119FD0A0D3_0_1_0.py.

[cfarrow]

Thank you.

[cfarrow]

I took a quick look at this today. The code generator appears to be doing the right thing. This is the relevant entry from the tlb:

[id(00000000), helpcontext(0x00010171)]
HRESULT Item(
                [in] VARIANT Index, 
                [out, retval] IAcadEntity** pItem);

The Add3DFace method also takes VARIANTs as input arguments. Are you having trouble with that method?

[cfarrow]

BTW, what version of comtypes are you using?

>>> import comtypes
>>> print comtypes.__version__

[cfarrow]

Another possibility is that Item is not being treated as a property because it is not marked as a property. It is given id 0, however, which usually signifies a default property.

[schernikov]

Thanks @cfarrow for looking at it.

Can not give you the version right now. I'm using comtypes indirectly through https://pypi.python.org/pypi/pyautocad/, which probably picked up latest comtypes from pypi as well.

Don't know much about MS COM in general or Autocad API. I just know that according to pretty simple logic of pyautocad it was succesfully using AcadBlock.Item(number) call to get an object back. See line 119 here https://github.com/reclosedev/pyautocad/blob/master/pyautocad/api.py#L119

[cfarrow]

That is helpful, thanks.

Can you check one more tweak for me? Do you still get the crash if you change the generated code to the following?

IAcadBlock._methods_ = [
    COMMETHOD([dispid(0), 'propget', helpstring(u'Gets the member object at a given index in a collection, group, or selection set')], HRESULT, 'Item',
              ( ['in'], VARIANT, 'Index' ),
              ( ['retval', 'out'], POINTER(POINTER(IAcadEntity)), 'pItem' )),

[cfarrow]

Another data point, this blog post shows that the VARIANT is not expected to be a POINTER. I cannot say why your workaround works.

[schernikov]

Chris, Still can not test the code at the moment with property flag as you are suggesting above. Sorry about that.

My thinking on your note regarding VARIANT and it's use in this blog post goes like this:

1. They are iterating over all blocks in active document, while in my case I was iterating over items inside of one block. Nevertheless, suspect that Item() API calls are similar in both cases.
2. Examples they provide don't show Item declaration. It could go like this:

void IAcadBlocks::Item(_variant_t & var);

i.e. they could have a reference to VARIANT there which is identical to

void IAcadBlocks::Item(_variant_t * var);

from compiled binary point of view. In other words, Item() still gets a pointer as it's argument. AFAIK, from ctypes perspective it could be done via byref() or pointer().

[cfarrow]

Indeed, they are calling different things. I checked, and as you suspected, the call signatures for accessing the blocks or items in a block are the same.

The pass-by-reference answer does explain things. What I was missing was how comtypes was supposed to know that it should pass the VARIANT by reference. I think I understand the reason. According to this bug, ctypes does not pass large structures (such as VARIANT) by reference on 64-bit Windows as it should. (So, ctypes should be handling this, not comtypes.) Are you running this on 64-bit python?

[schernikov]

Yes, it is executed via 64 bit Python.

My assumption was that such info as a method of passing arguments into functions is part of tlb definitions (as i mentioned, I have very little knowledge in this area). You are saying it's not the case, right?

Well, then comtypes may rely on it's knowledge of structure size and explicitly set direct, byref or pointer types depending on argument size and platform type (32 or 64 bit). I know, that would be something ctypes suppose to do, but since it has bugs comtypes may deal with it from it's end.

BTW, I wonder if this kind of problem is specific only to VARIANT or any kind of structure with large sizeof?

[cfarrow]

The typelib should define the proper calling signature, and in light of issue20160 it probably does. Passing a POINTER(VARIANT) and VARIANT in the first 4 arguments of a function should look the same at the binary interface level, but ctypes does not do this correctly. By changing the generated module as you have, you essentially sidestep this issue.

This problem could be handled by comtypes. This would take a bit of time. I'll need to create test library and test case that verifies the problem, and then figure out the fix. (For the sake of maintaining the project, I would rather not test against AutoCAD.)

To answer your last question, this does indeed affect other aggregate types. Any struct or union that is not 8, 16, 32, or 64 bits will need special treatment. It would be nice to rely on the ctypes fix, but that fix will never come to Python 2.