microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 238 forks source link

Bug: In some situations, calling get_weak() may over-release the "outer" object #1431

Closed JesseCol closed 1 week ago

JesseCol commented 1 month ago

Version

2.0.240405.15

Summary

There seems to be an issue where get_weak() will over-release the "outer" object when:

In the debugger, for the repro case it appears get_weak()'s net effect on the refcounts is that the inner gets +1 ref count and the "this" gets -1 ref count.

Here's a repro project: GetWeakRefCount.zip

For best results, enable pageheap for the process before running (as admin: "gflags.exe -i ThermometerCoreApp.exe +hpa").

It is UWP because it's based on the "ThermometerWRC" sample project.

Reproducible example

No response

Expected behavior

I'd expect get_weak() not to effect the strong ref count of the inner or outer objects.

Actual behavior

In the repro case, when I call get_weak() I'm seeing the outer object lose a strong ref, and the inner object gain a strong ref.

Additional comments

No response

kennykerr commented 1 month ago

Spent a bit of time debugging this on my end. A few thoughts before I get back to work and forget all about this. 😉

Here's a more minimal example of open composability in C++/WinRT for debugging purposes only:

https://gist.github.com/kennykerr/2a5dd202274f49031adbd50e02d8b4d0

github-actions[bot] commented 3 weeks ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

acoates-ms commented 3 weeks ago

To work around this issue, would it be possible to implement a base composable type in something other than C++/WinRT, then use C++/WinRT to compose types on top of that? Similar to what Xaml does?

JesseCol commented 3 weeks ago

Thanks Andrew, I do expect that would avoid the problem.

I forgot to mention, the workaround I'm using is to use make_weak() instead of get_weak(). This goes through the outer object (rather than the inner) and the refcounts appear to be updated correctly.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.