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

[Bug]: "Almost always use this as the left hand side of a WhenAny call" is misleading #835

Open
SuperJMN opened this issue Mar 1, 2024 · 2 comments
Labels

Comments

@SuperJMN
Copy link

SuperJMN commented Mar 1, 2024

Hello!

I've been following the recommendations in the docs regarding the usage of WhenAny (https://www.reactiveui.net/docs/guidelines/framework/use-this-on-left-of-whenany.html) but, recently, we faced a severe memory leak. I did some research and the offending line causing it was like this:

this.WhenAnyValue(x => x.Service.SomeProperty).BindTo(this, x => x.MyProperty);

...where Service was a reference to a instance that was injected via constructor. You can check the original file here.

The whole team was pretty confused, because the docs especifically mention says this is the good way to proceed when we want to monitor externally created dependencies, regardless of their lifetime:

"Almost always use this as the left hand side of a WhenAny call"
"Why? The lifetime of dependency is unknown - if it is a singleton it could introduce memory leaks into your application."

I started to wonder if calling WhenAnyValue on this to watch "foreign" objects without explicitly disposing subscriptions was safe.

If it's not safe, then the docs are misleading: It looks like watching properties of other dependencies (that can potentially have bigger lifetimes) doesn't need to manage subscriptions explicitly.

Expected behavior

The docs should be clear about disposing subscriptions to avoid memory leaks.

UPDATE!

I've created a sample to reproduce the memory leak in our application, but even though the code is essentially the same, the memory leak doesn't happen. I'm so confused.

This is the minimal repro:
https://github.com/SuperJMN-Tutorials/RxUI-MemoryLeak-Test

If you run the application, you'll see that every time you click the button, a new list of items are displayed. Each item is subscribed to a "Service" that is a Singleton. However, I don't do any subscription management. I don't dispose anything, yet the memory usage doesn't increase over time, and analyzing with JetBrains DotMemory, it gives the correct figures.

What's going on? How to handle subscriptions? when is it safe to avoid disposing explicitly and when is not? I'm (we are) riddled by this.

@SuperJMN SuperJMN added the bug label Mar 1, 2024
@SuperJMN SuperJMN changed the title [Bug]: "Almost always use this as the left hand side of a WhenAny call" might be misleading [Bug]: "Almost always use this as the left hand side of a WhenAny call" is misleading Mar 1, 2024
@anaisbetts
Copy link
Member

It's still good advice, but it needs some additional qualifiers, if you alias a passed-in variable:

public Service SomeOtherThing
public MyCoolViewModel(Service someOtherThing) {
  SomeOtherThing = someOtherThing;
  
  // ...
  
  this.WhenAny(x => x.SomeOtherThing.Foo).Subscribe();
}

You've reached through an object that was given to you, but made it look like you didn't! In that case, you need to treat it as if you just wrote:

someOtherThing.WhenAny(....)

(i.e. you need to think about subscription lifetimes and Disposing)

@SuperJMN
Copy link
Author

SuperJMN commented Mar 1, 2024

It's still good advice, but it needs some additional qualifiers, if you alias a passed-in variable:

public Service SomeOtherThing
public MyCoolViewModel(Service someOtherThing) {
  SomeOtherThing = someOtherThing;
  
  // ...
  
  this.WhenAny(x => x.SomeOtherThing.Foo).Subscribe();
}

You've reached through an object that was given to you, but made it look like you didn't! In that case, you need to treat it as if you just wrote:

someOtherThing.WhenAny(....)

(i.e. you need to think about subscription lifetimes and Disposing)

Yes, that makes sense, but then how is this working? https://github.com/SuperJMN-Tutorials/RxUI-MemoryLeak-Test?
Please, notice that this minimal repro doesn't have any memory leak, but it doesn't do any subscription management.

GitHub
Contribute to SuperJMN-Tutorials/RxUI-MemoryLeak-Test development by creating an account on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants