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 bad refcount management in ComWrappers WeakReference test. #37022

Merged
merged 1 commit into from
May 26, 2020

Conversation

jkoritzinsky
Copy link
Member

When the ComWrappers instance is called by the runtime, we can't assume that we take control of the reference. The contract is that we don't own the reference we are passed when called globally, which I got wrong in this test.

Fixes #36970

@jkoritzinsky jkoritzinsky added test-bug Problem in test source code (most likely) area-Interop-coreclr labels May 26, 2020
@jkoritzinsky jkoritzinsky added this to the 5.0 milestone May 26, 2020
@AaronRobinsonMSFT
Copy link
Member

@elinor-fung

@AaronRobinsonMSFT
Copy link
Member

The contract is that we don't own the reference we are passed when called globally

Can you unpack that statement. This should be true for any scenario where ComWrappers.CreateObject() is called.

@jkoritzinsky
Copy link
Member Author

Technically if you’re calling your ComWrappers instance directly instead of using any of the “global instance” features, you can ignore that contract and change it for your usage (let your CreateObject impl take ownership on both the impl and caller side) and things will still work fine, which is why I initially didn’t notice that I had broken the contract. Once you’re using any of the global features, you have to follow the contract.

@jkoritzinsky jkoritzinsky merged commit 2586a5c into dotnet:master May 26, 2020
@jkoritzinsky jkoritzinsky deleted the fix-weakreference-test branch May 26, 2020 22:03
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Interop-coreclr test-bug Problem in test source code (most likely)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure: Interop\\COM\\ComWrappers\\WeakReference\\WeakReferenceTest\\WeakReferenceTest.cmd
2 participants