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

Documentation should mention that it is unnecessary to release returned objects #612

Open
bytesized opened this issue Sep 4, 2024 · 3 comments
Labels
shared_info use cases, tips and troubleshoots

Comments

@bytesized
Copy link

Coming from C++ COM, where very simple COM usage looks like

IFoo* foo;
HRESULT hr = CoCreateInstance(rclsid, pUnkOuter, dwClsContext, riid, (void**) &foo);
if (SUCCEEDED(hr)) {
  // do something with foo...
  foo->Release()
}

it was highly confusing to try out this library for the first time and do something like this

>>> try:
...   foo = CreateObject(progid)
... except:
...   pass
... else:
...   # Do something with foo
...   foo.Release()
...
0
>>> foo = None
Exception ignored in: <function _compointer_base.__del__ at 0x0000020D7D3FC3A0>
Traceback (most recent call last):
  File "C:\Users\bytesized\sandbox\python\venv\lib\site-packages\comtypes\_post_coinit\unknwn.py", line 386, in __del__
    self.Release()
  File "C:\Users\bytesized\sandbox\python\venv\lib\site-packages\comtypes\_post_coinit\unknwn.py", line 534, in Release
    return self.__com_Release()  # type: ignore
OSError: exception: access violation writing 0x0000000000000000

I tried looking through the documentation for CreateObject and using the search bar to look for "Release" and a few similar search terms, but couldn't find anything related.

This feels weird since AddRef and Release are standard COM methods and are made available to the interface consumer. They just don't quite seem to work as expected. Normally, things are handed to you (by CoCreateInstance, for example) already AddRefed. So you have to Release them, plus an additional Release for every additional AddRef. But this implementation does not match this; it's strictly one Release per AddRef.

It seems like if this COM library is going to differ from the usual COM conventions, the documentation should at least mention it.

@junkmd
Copy link
Collaborator

junkmd commented Sep 6, 2024

Hi, @bytesized.

Indeed, those who are familiar with COM and are used to explicitly releasing COM interface pointers that are no longer in use might find this package's implementation weird.

The originator of this package likely aimed to make it easier for users to work with the COM framework by using atexit and __del__ to trigger cleanup operations, so that users would not need to manually release COM pointers that are no longer in use.

Through the functionality of metaclasses, when an instance of an IUnknown Python object is destroyed and __del__ is called, Release is automatically triggered.

class _compointer_base(c_void_p, metaclass=_compointer_meta):
"base class for COM interface pointer classes"
def __del__(self, _debug=logger.debug):
"Release the COM refcount we own."
if self:
# comtypes calls CoUninitialize() when the atexit handlers
# runs. CoUninitialize() cleans up the COM objects that
# are still alive. Python COM pointers may still be
# present but we can no longer call Release() on them -
# this may give a protection fault. So we need the
# _com_shutting_down flag.
#
if not type(self)._com_shutting_down:
_debug("Release %s", self)
self.Release()

Additionally, it's worth mentioning that when the _shutdown callback function registered with atexit is invoked, comtypes not only calls the Release method on each COM interface pointer but also calls CoUninitialize.

atexit.register(_shutdown)

def _shutdown(
func=_ole32_nohresult.CoUninitialize,
_debug=logger.debug,
_exc_clear=getattr(sys, "exc_clear", lambda: None),
):
# Make sure no COM pointers stay in exception frames.
_exc_clear()
# Sometimes, CoUninitialize, running at Python shutdown,
# raises an exception. We suppress this when __debug__ is
# False.
_debug("Calling CoUninitialize()")
if __debug__:
func()
else:
try:
func()
except WindowsError:
pass
# Set the flag which means that calling obj.Release() is no longer
# needed.
if _cominterface_meta is not None:
_cominterface_meta._com_shutting_down = True
_debug("CoUninitialize() done.")

class _cominterface_meta(type):
"""Metaclass for COM interfaces. Automatically creates high level
methods from COMMETHOD lists.
"""
_case_insensitive_: bool
_iid_: GUID
_methods_: List[_ComMemberSpec]
_disp_methods_: List[_DispMemberSpec]
# This flag is set to True by the atexit handler which calls
# CoUninitialize.
_com_shutting_down = False

It is true that there is no documentation indicating that users generally do not need to manually call Release, CoInitialize at startup, or CoUninitialize at shutdown.
We welcome PRs to add explanations about these behaviors, so please feel free to contribute.

@junkmd junkmd added the shared_info use cases, tips and troubleshoots label Sep 6, 2024
@junkmd
Copy link
Collaborator

junkmd commented Sep 6, 2024

In the past, changes were made in #215 to define _shutdown as a private function, allowing cleanup operations to be unregistered from atexit.
By temporarily modifying IUnknown._com_shutting_down manually, it is also possible to prevent Release from being called when __del__ is invoked.

However, I should mention that if alternative cleanup operations are not performed or the modified class attributes are not restored, it will be necessary to manually and properly release the COM pointers present in the running Python interpreter.

@junkmd
Copy link
Collaborator

junkmd commented Oct 6, 2024

As a byproduct of investigating the issues with Python 3.13 and metaclasses in #618, I came across several projects that apply workarounds to _compointer_base for handling pointer release.

Some of these projects used super(_compointer_base, obj).value to retrieve the raw pointer, while others patched _compointer_base.__del__.

Since version 1.4.5, the definition of _compointer_base was moved to comtypes._post_coinit.unknwn, making these imports and references a bit tricky.

In #215, the behavior at shutdown was made customizable for users by turning the function hooked to atexit into a private function, instead of removing it from the comtypes/__init__.py namespace.

Similarly, it might benefit users if _compointer_base were imported into comtypes/__init__.py, making it accessible via from comtypes import _compointer_base or comtypes._compointer_base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shared_info use cases, tips and troubleshoots
Projects
None yet
Development

No branches or pull requests

2 participants