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

[API Proposal]: (Re)introduce WeakAttribute that makes a field behave as if it were a WeakReference. #68702

Open
Tracked by #14548
rolfbjarne opened this issue Apr 29, 2022 · 8 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Apr 29, 2022

Background and motivation

UI toolkits (such as Xamarin.iOS) often have to mirror an object graph between managed and native code, and this makes it very easy for consumers of such toolkits to run into circular references the GC can't resolve, and the eventual memory leaks.

The solution is usually to use weak references to break the circular references, but today that's very cludgy to write in C#:

  • A field has to change type from object to WeakReference<object>.
  • All usages have to be explicit when they want to use the target of the weak reference.

It would be much preferable to just add an attribute onto a field, and then .NET would wave a wand and make everything work as expected.

This was already implemented in Mono some time ago, and used (with success) in Xamarin.iOS.

The semantics would be that fields with this attribute:

  • Are not considered GC references.
  • Are nulled out when the GC collects them (this would mean that weak fields are always nullable).

It's out of scope for this issue, but this attribute could also back a weak modifier for C# (which has already been proposed). If C# support is added, nullability analysis could be improved to report that weak fields can become null at any point:

public class Obj {
	[Weak]
	public object Value;

	public void DoSomething ()
	{
		if (Value is null)
			return;
		Console.WriteLine (Value.ToString ()); // <!-- The C# compiled could warn that 'Value' can be null here.
	}
}

It could also be used for captured variables in lambas somehow. Not sure how the syntax would be, but maybe something like this:

public void Func ()
{
	var obj = new object ();
	var action = new Action (() =>
	{
		weak var capturedObject = obj;
		// capturedObject might become null at any point.
	});
}

API Proposal

namespace System {
    [AttributeUsage (AttributeTargets.Field)]
    public sealed class WeakAttribute : Attribute
    {
    }
}

The namespace is not really important for this discusion (Mono had it in System, so that's where I put it here).

API Usage

This illustrates one common reason for wanting weak fields, in a tree structure where leaf nodes have a reference to a node further up the tree:

public class Root {
	public Leaf Leaf;

	public void DoSomething ()
	{
		Leaf = new Leaf (this);
	}
}
public class Leaf {
	[Weak]
	public Root Root;
	public Leaf (Root root)
	{
		Root = root;
	}
}

A real-world sample would be much more complex, often involving object trees that are mirrored in or from native code (quite frequent in some UI toolkits).

My case is Xamarin.iOS, but other people have mentioned this could be useful for XAML UI apps, WPF, etc: dotnet/roslyn#13950 (comment)

Alternative Designs

Use the existing WeakReference class.

Risks

It's already been implemented in Mono (and it worked just fine), so it's a tested concept.

It should also not have an impact on code not using weak fields.

@rolfbjarne rolfbjarne added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 29, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 29, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Apr 29, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

UI toolkits (such as Xamarin.iOS) often have to mirror an object graph between managed and native code, and this makes it very easy for consumers of such toolkits to run into circular references the GC can't resolve, and the eventual memory leaks.

The solution is usually to use weak references to break the circular references, but today that's very cludgy to write in C#:

  • A field has to change type from object to WeakReference<object>.
  • All usages have to be explicit when they want to use the target of the weak reference.

It would be much preferable to just add an attribute onto a field, and then .NET would wave a wand and make everything work as expected.

This was already implemented in Mono some time ago, and used (with success) in Xamarin.iOS.

The semantics would be that fields with this attribute:

  • Are not considered GC references.
  • Are nulled out when the GC collects them (this would mean that weak fields are always nullable).

It's out of scope for this issue, but this attribute could also back a weak modifier for C# (which has already been proposed). If C# support is added, nullability analysis could be improved to report that weak fields can become null at any point:

public class Obj {
	[Weak]
	public object Value;

	public void DoSomething ()
	{
		if (Value is null)
			return;
		Console.WriteLine (Value.ToString ()); // <!-- The C# compiled could warn that 'Value' can be null here.
	}
}

It could also be used for captured variables in lambas somehow. Not sure how the syntax would be, but maybe something like this:

public void Func ()
{
	var obj = new object ();
	var action = new Action (() =>
	{
		weak var capturedObject = obj;
		// capturedObject might become null at any point.
	});
}

API Proposal

namespace System {
    [AttributeUsage (AttributeTargets.Field)]
    public sealed class WeakAttribute : Attribute
    {
    }
}

The namespace is not really important for this discusion (Mono had it in System, so that's where I put it here).

API Usage

This illustrates one common reason for wanting weak fields, in a tree structure where leaf nodes have a reference to a node further up the tree:

public class Root {
	public Leaf Leaf;

	public void DoSomething ()
	{
		Leaf = new Leaf (this);
	}
}
public class Leaf {
	[Weak]
	public Root Root;
	public Leaf (Root root)
	{
		Root = root;
	}
}

A real-world sample would be much more complex, often involving object trees that are mirrored in or from native code (quite frequent in some UI toolkits).

My case is Xamarin.iOS, but other people have mentioned this could be useful for XAML UI apps, WPF, etc: dotnet/roslyn#13950 (comment)

Alternative Designs

Use the existing WeakReference class.

Risks

It's already been implemented in Mono (and it worked just fine), so it's a tested concept.

It should also not have an impact on code not using weak fields.

Author: rolfbjarne
Assignees: -
Labels:

api-suggestion, area-System.Runtime, untriaged

Milestone: -

@stephentoub
Copy link
Member

stephentoub commented Apr 29, 2022

In your experience, was this purely an ergonomics / usability improvement, or did it also yield measurable performance benefits?

Looking at WPF, for example, its Dependent struct has two WeakReference fields, resulting in a fair number of WeakReference allocations that would presumably go away with such a feature. I'm wondering if this would move the needle on pause times and the like.

@rolfbjarne
Copy link
Member Author

For Xamarin.iOS, the motivation was ergonomics / usability, as "circular references -> memory leaks -> unhappy customers" was our motivation, and we wanted to improve that scenario. As far as I know, no performance measurements were done.

That said, if weak fields would be faster than WeakReferences, that would certainly be nice :)

@teo-tsirpanis
Copy link
Contributor

c.c. @dotnet/gc

@jkotas
Copy link
Member

jkotas commented Apr 29, 2022

This is the PR with the original implementation in mono/mono: mono/mono#5972 . We have discussed what it would take to implement this in CoreCLR at that time as well (I lost the paper trail).

The weak fields create a weak GC handle and attach it to the object instance through runtime magic (similar to how e.g. contended Monitor locks are attached to the object). This runtime magic is not particularly efficient, at least in CoreCLR. The intrinsic weak fields are unlikely to have significantly better performance characteristics compared to what you can do manually by creating GC handles on your own. Also, the intrinsic weak fields would have sharp edges. The GC handles in CoreCLR GC need write barriers that are different from regular write barrier, so we would likely need to prohibit taking ref of the weak field somehow.

I think it would be better to look at what it would take to source generate the boilerplate for weak fields so that there is no runtime magic involved.

@sakno
Copy link
Contributor

sakno commented May 5, 2022

The attribute could change the behavior of all methods of particular class. Many developers doesn't store the field as local variable inside of the method. As a result, GC can do its job in any place inside of the method and produce NullReferenceException. Existing WeakReference provides explicit control over the lifetime of the referenced object. Typically, developer needs to store strong reference explicitly to the local variable.

@tannergooding tannergooding added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jul 15, 2022
@tannergooding tannergooding added this to the Future milestone Jul 15, 2022
@SF-Simon
Copy link

SF-Simon commented Mar 6, 2023

This suggestion was put forward in 2016, and no progress has been seen so far. ( dotnet/roslyn#13950 )
I think most users, like me, don't care how it is implemented, but just want to simplify the code and make it easy to read. Many programing already support the weak keyword, but C-Sharp is not one of them.

It is not how special grammar is, it is called "high-level language". Simplification and acceptance are the main chords of true modernization.

I hope we can be positive about this proposal, and we don't have to wait until 2024 to see the voice of approval. Maybe I don't know when I will go fishing in the water, and I can only look from heaven, because... It's too far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

7 participants