Skip to content

ext/com_dotnet: release the held IUnknown in com_get_active_object()#22378

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:com-dotnet-active-object-release
Open

ext/com_dotnet: release the held IUnknown in com_get_active_object()#22378
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:com-dotnet-active-object-release

Conversation

@iliaal

@iliaal iliaal commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

The cleanup block guards on unk but releases obj, so a successful GetActiveObject() releases the IDispatch proxy twice and never releases the IUnknown it returned, leaking it. Release unk so each interface pointer is released exactly once. Windows-only; no test, since it needs a live Running Object Table entry.

The cleanup block guarded on `unk` but released `obj`, so a successful
GetActiveObject() released the IDispatch proxy twice (once already under
the obj guard) and never released the IUnknown it returned, leaking it.
Release `unk` so each interface pointer is released exactly once.

@devnexen devnexen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This also fixes a NULL-deref crash, not just a leak. On the QueryInterface failure branch (com_com.c:310-313), obj stays NULL while unk is valid, so the old if (unk)
IUnknown_Release(obj) at com_com.c:325 releases NULL through a NULL vtable and crashes instead of throwing. IUnknown_Release(unk) is guarded non-NULL, so it's correct
there too. Worth a line in the commit message IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants