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

Fix dynamic property put. #1641

Closed
wants to merge 1 commit into from

Conversation

dankob00
Copy link

Since version 224 of pywin32 the dynamic property put is broken. It seems that a fix for a regression broke it (see issues #1427 and #1199, and the corresponding change in commit 1350d43).
The severity of this issue increased recently as the last working version 223 is not available for the newest Python release 3.8 and 3.9.

The fix checks for the invoke type for property put and returns it accordingly. This prevents the exception reported in #1427 and successfully sets the property's value.

@dankob00 dankob00 marked this pull request as ready for review December 15, 2020 16:20
@dankob00
Copy link
Author

I would appreciate it to get some feedback about this pull request. If there is anything that is unclear or missing for merging this pull request I'm happy to update the pull request or provide additional information. I'm looking forward to get the issue fixed as it blocks us from upgrading to newer Python versions.

@mhammond
Copy link
Owner

Sorry for the delay, but this code is somewhat tricky and has changed lots over time. I need to find the time to work out what exactly changed in 224, and why, and how this impacts it. This change has no tests, so I've no idea what regressions it might cause.

I'll try and find some time soon, and will do so before cutting a new release, but I've been very busy. The best thing you can do to move this forward is to do some of the above.

@dankob00
Copy link
Author

dankob00 commented Jan 28, 2021

Here is what I found out so far:

I also searched for existing tests and could only find this one: testDynamic.py. As far as I understand the code, it dynamically registers a python object as COM object and performs some tests on it. Unfortunately, this object does not behave the same as "real" COM objects as the Dispatch method in dynamic.py can not get the type info (IDispatch.GetTypeInfo()). Therefore, _lazydata_ in CDispatch is None and __setattr__ takes another route as it does for "real" COM objects.

I'm stuck here as I don't know enough about the code and COM to extend this test to cover the proposed fix.

@mhammond
Copy link
Owner

(Oops - I accidentally put this into #1427 instead of here, so re-pasting here!)

Sorry for the delay - I've started looking at this and things are a bit of a mess around this code. I've added some comments about that and I'm slowly cleaning things up a little so I can re-understand how things work :) It might take me a bit longer to untangle and I've a few other commits queued up in that direction. Please let me know if you are able to help me test things when I get a little further along - thanks!

@dankob00
Copy link
Author

Thanks for looking into it! For sure I will help testing.

@mhammond
Copy link
Owner

If you visit https://github.com/mhammond/pywin32/runs/2225136583 you should be able to find binaries of the most recent commit I made. What would be super helpful would be if you can adjust your patch, so that the condition you add is:

  •     elif entry.desc.desckind == pythoncom.DESCKIND_VARDESC and varkind == pythoncom.VAR_DISPATCH and invoke_type == pythoncom.INVOKE_PROPERTYPUT:
    

(ie, an additional check for entry.desc.desckind == pythoncom.DESCKIND_VARDESC) and make sure it still works as expected. If that doesn't fix your use-case then I will remain very confused, but if it does, I can at least rationalize about why it does work.

Again, note that these changes will only work with a very recent binary.

Thanks

@dankob00
Copy link
Author

I fetched the build from https://github.com/mhammond/pywin32/runs/2225136583, applied my patch with your extension (see rebased commit c15043c) and gave it a try (Python 3.6 x64).
entry.desc.desckind in fact is pythoncom.DESCKIND_VARDESC and the property put works as expected with the fix!

Please, let me know if you need additional information.

@mhammond
Copy link
Owner

Thanks for checking. I think I'll probably end up doing things slightly differently, but that's extremely helpful. Are you able to find the property definition in the type library for the property in question? When running the test suite (albeit without Office etc installed) I never hit that code with a VARDESC - it's always a FUNCDESC - even though the tests do have properties - so I'm finding that somewhat confusing and I'm still hoping to get a test case that covers your scenario

@dankob00
Copy link
Author

The setter does not fail for a specific property or properties of a certain type but for all properties of a certain object. The peculiarity of that object might be that it is returned as a plain IDispatch pointer:

[id(DISPID_HWC_SPECIFIC), helpcontext(HIDHWC_Specific), helpstring("Get hardware configuration specific object."), readonly] IDispatch* Specific;

Setting properties on descendants of this Specific object results in the observed behavior:

hwc = doc.configuration.Specific
hwc.TimeSource.TriggerPTP.Profile = "Power"

I hope this information is of any help...

@mhammond
Copy link
Owner

mhammond commented Apr 2, 2021

Thanks - do you happen to have the typeinfo available for that Specific object? Even testing MSOffice tests I can't see a VARDESC

@dankob00
Copy link
Author

dankob00 commented Apr 6, 2021

This application uses OLE. doc is a container with an embedded configuration object. When property Specific is accessed, the OLE control is activated and its IDispatch interface is queried and returned. The actual interface of the object is called IAutoApp and is defined in the same way as other interfaces with some read-only properties.

@mhammond
Copy link
Owner

I think I fixed this in af54b27, so I'm going to close this.

@mhammond mhammond closed this May 29, 2021
@dankob00
Copy link
Author

I quickly verified the fix in our environment and it works as expected.
Thanks a lot for the fix and the release!

@dankob00 dankob00 deleted the fix-dynamic-property-put branch May 31, 2021 06:08
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

Successfully merging this pull request may close these issues.

None yet

2 participants