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

[server\__init__.py] Use OleDLL instead of oledll #769

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

moi15moi
Copy link
Contributor

See #735

Comment on lines 75 to 80
_RegisterActiveObject.argtypes = [
POINTER(IUnknown),
REFCLSID,
DWORD,
POINTER(DWORD),
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this has the same issue as #759 (comment).
The reason the CI is passing is that there are no tests for this function.

I recommend changing POINTER(IUnknown) to c_void_p.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recommend changing POINTER(IUnknown) to c_void_p.

Before doing that, I think it would be worth it to create a test that would cover the RegisterActiveObject and RevokeActiveObject function, but I have no knownledge about comtype server.

So, I'll keep this PR open so that if tests for these functions are added in the future, we can continue the investigation

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed.

Without untangling the server-side implementation, progress on python/cpython#127369 cannot be made.

There is documentation on COM/OLE published under the MIT license.
https://github.com/kraigb/InsideOLE/

If you can allocate resources to this issue and learn something from reading that documentation, please feel free to share it.

Copy link
Collaborator

@junkmd junkmd Jan 26, 2025

Choose a reason for hiding this comment

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

This is what I know about these function and my observations.

These are simple wrapper functions around the GetRunningObjectTable API and the IRunningObjectTable interface 1.

RegisterActiveObject corresponds IRunningObjectTable::Register.
RevokeActiveObject corresponds IRunningObjectTable::Revoke method.

The tests for IRunningObjectTable is existing in this project.

class Test_IRunningObjectTable(unittest.TestCase):
def test_register_and_revoke_item_moniker(self):
vidctl = CreateObject(msvidctl.MSVidCtl, interface=msvidctl.IMSVidCtl)
item_id = str(GUID.create_new())
mon = _create_item_moniker("!", item_id)
rot = _create_rot()
bctx = _create_bctx()
self.assertEqual(mon.IsRunning(bctx, None, None), hresult.S_FALSE)
dw_reg = rot.Register(ROTFLAGS_ALLOWANYCLIENT, vidctl, mon)
self.assertEqual(mon.IsRunning(bctx, None, None), hresult.S_OK)
self.assertEqual(f"!{item_id}", mon.GetDisplayName(bctx, None))
self.assertEqual(rot.GetObject(mon).QueryInterface(msvidctl.IMSVidCtl), vidctl)
rot.Revoke(dw_reg)
self.assertEqual(mon.IsRunning(bctx, None, None), hresult.S_FALSE)

From the changelog, they were added without any tests, and I can’t find any use cases for these functions in the server.

comtypes/CHANGES.txt

Lines 624 to 627 in 8945446

2008-11-26 Thomas Heller <[email protected]>
* Added untested code to comtypes.server: RegisterActiveObject()
and RevokeActiveObject(), plus some flags.

It’s puzzling that these functions are not defined near GetActiveObject and instead reside on the server side.
Additionally, it’s strange that RegisterActiveObject, which is determining which instance is the active object of a class, is not wrapped to accept punk.

Perhaps these functions might be marked as deprecated and adding alternative functions somewhere else in this project.

What do you think?

Footnotes

  1. InsideOLE.pdf, p489.

Copy link
Collaborator

@junkmd junkmd Jan 27, 2025

Choose a reason for hiding this comment

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

I’ve figured out the use case for RegisterActiveObject.

When we pass a SomeComObject instance (let’s say it's assigned to the variable c) to this function (RegisterActiveObject(c)), the instance o obtained via o = c.QueryInterface(ISomeComInterface) becomes the active object of that class.
We can then retrieve this instance using p = GetActiveObject(SomeComObject), and the condition o == p holds true.

This behavior is indeed meaningful as a utility function.

I’ve written a test for this and will submit a PR.

(P.S. That said, I think having a version of RegisterActiveObject that accepts punk would also be convenient. In the future, we might see the addition of a RegisterActiveObject on the client side that accepts arguments differing from those on the server side.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I opened #776.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#776 has been merged.

Please rebase this PR’s branch and check the CI pipeline test results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any doc to build source\CppTestSrv?
I tried with Visual Studio, but when I run the command server.exe /RegServer, this assert always fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like you said, I changed POINTER(IUnknown) to c_void_p for _RegisterActiveObject.

To know why, see this pipeline where I used POINTER(IUnknown): https://github.com/enthought/comtypes/actions/runs/13022699670/job/36326427772

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any doc to build source\CppTestSrv?

There is no documentation on building the test doubles, but I believe it might work if you replicate the CI pipeline workflow.

- name: Set up MSVC
uses: ilammy/msvc-dev-cmd@v1
- name: Build and register the OutProc COM server
run: |
cd source/CppTestSrv
nmake /f Makefile
./server.exe /RegServer

Comparing the Visual Studio setup provided by ilammy/msvc-dev-cmd with your environment might reveal some insights.

@moi15moi moi15moi force-pushed the Use-WinDLL/OleDLL-for-server_init branch from 8f218e4 to 9e2a784 Compare January 29, 2025 01:21
junkmd added a commit to junkmd/pywinauto that referenced this pull request Jan 29, 2025
@junkmd junkmd added this to the 1.4.10 milestone Jan 29, 2025
Copy link
Collaborator

@junkmd junkmd left a comment

Choose a reason for hiding this comment

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

Thanks!

@junkmd junkmd merged commit b7e5bcc into enthought:main Jan 29, 2025
50 checks passed
@moi15moi moi15moi deleted the Use-WinDLL/OleDLL-for-server_init branch January 29, 2025 11:02
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.

2 participants