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 bug in COM interface chain support #3060

Merged

Conversation

sivadeilra
Copy link
Collaborator

The definition of a COM interface may inherit from another interface. These are known as "interface chains". The #[implement] macro allows designers to specify only the minimal set of interface chains that are needed for a given COM object implementation. The #[implement] macro (and the #[interface] macro) work together to pull in the implementations of all interfaces along the chain.

Unfortunately there is a bug in the implementation of QueryInterface for interface chains. The current QueryInterface implementation will only check the IIDs of the interfaces at the root of the chian, i.e. the "most-derived" interface. QueryInterface will not search the IIDs of interfaces that are in the inheritance chain.

This bug is demonstrated (detected) by the new unit tests in crates/tests/implement_core/src/com_chain.rs. This PR fixes the bug by generating an fn matches() method that checks the current IID and then checks the parent interface (if any) by calling its match() method. This fixes the unit test.

The definition of a COM interface may inherit from another interface.
These are known as "interface chains". The `#[implement]` macro allows
designers to specify only the minimal set of interface chains that are
needed for a given COM object implementation. The `#[implement]` macro
(and the `#[interface]` macro) work together to pull in the
implementations of all interfaces along the chain.

Unfortunately there is a bug in the implementation of `QueryInterface`
for interface chains. The current `QueryInterface` implementation will
only check the IIDs of the interfaces at the root of the chian, i.e.
the "most-derived" interface. `QueryInterface` will not search the IIDs
of interfaces that are in the inheritance chain.

This bug is demonstrated (detected) by the new unit tests in
`crates/tests/implement_core/src/com_chain.rs`. This PR fixes the bug
by generating an `fn matches()` method that checks the current IID and
then checks the parent interface (if any) by calling its `match()`
method. This fixes the unit test.
@kennykerr
Copy link
Collaborator

'matches' definitely needs to check parent IIDs.

@sivadeilra sivadeilra requested review from riverar and dpaoliello May 29, 2024 00:58
@sivadeilra sivadeilra merged commit dbc3932 into microsoft:master May 29, 2024
90 checks passed
@sivadeilra sivadeilra deleted the user/ardavis/fix-interface-chains branch May 29, 2024 21:13
// QueryInterface to work correctly for all interfaces in an inheritance chain, e.g.
// IFoo3 derives from IFoo2 derives from IFoo.
//
// We avoid matching IUnknown because object identity depends on the uniqueness of the IUnknown pointer.
Copy link
Collaborator

@kennykerr kennykerr Jun 4, 2024

Choose a reason for hiding this comment

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

May be worth adding a test for this. #3069

mati865 pushed a commit to mati865/windows-rs that referenced this pull request Jun 15, 2024
The definition of a COM interface may inherit from another interface.
These are known as "interface chains". The `#[implement]` macro allows
designers to specify only the minimal set of interface chains that are
needed for a given COM object implementation. The `#[implement]` macro
(and the `#[interface]` macro) work together to pull in the
implementations of all interfaces along the chain.

Unfortunately there is a bug in the implementation of `QueryInterface`
for interface chains. The current `QueryInterface` implementation will
only check the IIDs of the interfaces at the root of the chian, i.e.
the "most-derived" interface. `QueryInterface` will not search the IIDs
of interfaces that are in the inheritance chain.

This bug is demonstrated (detected) by the new unit tests in
`crates/tests/implement_core/src/com_chain.rs`. This PR fixes the bug
by generating an `fn matches()` method that checks the current IID and
then checks the parent interface (if any) by calling its `match()`
method. This fixes the unit test.

Co-authored-by: Arlie Davis <ardavis@microsoft.com>
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.

5 participants