Bug 18865 - WeakReference.TryGetTarget behavior is incorrect
Summary: WeakReference.TryGetTarget behavior is incorrect
Alias: None
Product: Runtime
Classification: Mono
Component: GC (show other bugs)
Version: unspecified
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Bugzilla
Depends on:
Reported: 2014-04-08 14:11 UTC by Jon Goldberger [MSFT]
Modified: 2018-03-14 00:10 UTC (History)
9 users (show)

See Also:
Tags: bugpool
Is this bug a regression?: ---
Last known good build:

Test Project (33.07 KB, application/zip)
2014-04-08 14:11 UTC, Jon Goldberger [MSFT]

Comment 1 Jon Goldberger [MSFT] 2014-04-08 14:11:54 UTC
From case:

We're using the latest Xamarin.iOS, and we're encountered a bug with the WeakRefence<T>.TryGetValue(), which returns an invalid value (true in this case), even if the target object has been collected.

Attached is a sample iOS application to demonstrate the issue, as well as a WPF application for the correct behavior.
Comment 3 Jon Goldberger [MSFT] 2014-04-08 14:13:27 UTC
I observed the same behavior loading the code into an iOS project in Xamarin Studio on the Mac.
Comment 5 Zoltan Varga 2014-04-10 17:15:04 UTC
I can't reproduce this, the test app prints:
ALIVE 0 on <n>, but TryGetTarget tell 0 on <n>
which I think is the correct behavior.
Comment 6 Andrei.N 2014-09-14 09:03:35 UTC
This is from Xamarin’s WeakReference<T>:

public bool TryGetTarget (out T target)
    if (!this.handle.IsAllocated) {
        target = (T)((object)null);
        return false;
    target = (T)((object)this.handle.Target);
    return target != null;

Isn’t it an issue that it first checks if it’s allocated and if it is, it THEN gets a reference to it?

Isn't it possible that between the two operations the garbage collector could reclaim the target?

Wouldn't be correct to write instead

public bool TryGetTarget (out T target)
    target = (T)((object)this.handle.Target);
    if (!this.handle.IsAllocated) {
        target = (T)((object)null);
    return target != null;
Comment 7 Ludovic Henry 2018-01-24 21:07:57 UTC
The implementation in corert and referencesource are closer to what's proposed in https://bugzilla.xamarin.com/show_bug.cgi?id=18865#c6. This should be fixed to avoid a possible race condition.
Comment 8 Rodrigo Kumpera 2018-03-13 23:52:52 UTC
No, the proposed change from #c6 doesn't make any difference as it's misunderstanding what the Allocated property means.

An allocated GCHandle can store null. Having the object being collected doesn't deallocate the gchandle. It only happens in the finalizer when the WeakReference itself gets collected.
Comment 9 Rodrigo Kumpera 2018-03-14 00:10:57 UTC
I cannot reproduce this issue any longer.

We fixed a lot of GC bugs in the past 4 years, please reopen if you can still reproduce it.

Note You need to log in before you can comment on or make changes to this bug.