Skip to content
This repository has been archived by the owner on Jul 3, 2022. It is now read-only.

Possible leak in CallerMap? #4

Closed
sindrenm opened this issue May 11, 2017 · 2 comments
Closed

Possible leak in CallerMap? #4

sindrenm opened this issue May 11, 2017 · 2 comments
Assignees

Comments

@sindrenm
Copy link

sindrenm commented May 11, 2017

Hi!

Really great stuff here, love how simple it is to use and how concise the code base is compared to other dependency injection libraries out there.

However, looking through your code, I noticed you’re extending WeakHashMap to allow for a simple caching mechanism of the last value stored, and I couldn’t help myself but wondering if keeping this reference outside of the “actual” map implementation could result in a leak somehow.
Imagine the following:

class HomeModule(private val activity: HomeActivity) {
  // EDITED: Typo had this return a HomeModule before ...
  val model by lazy { HomeModel(activity) }
}

Now, in our activity:

class HomeActivity : Activity, Injects<HomeModule> {
  private val model by required { model }
}

So far so good. However, when we move away from the HomeActivity, and given that we don’t do any injection in our next activity, then there will be a hard reference to the HomeModule, and therefore also the HomeActivity, which won’t get collected by the garbage collector.

I suppose it's only a valid issue for the last stored key/value pair, seeing as you're extending WeakHashMap.I suspect that's also why you did just that.

Now, if you already thought about this for the last key/value pair as well and have a solution already in your codebase, then please correct me. :smile:

@gouline gouline self-assigned this May 15, 2017
@gouline
Copy link
Owner

gouline commented May 15, 2017

Hi @sindrenm! Thanks for your issue.

Correct me if misunderstood your question, but the CallerMap doesn't actually store a reference to the HomeModule itself, it only stores a reference to the Kapsule<HomeModule>, which is just a lightweight injector. It never stores the module during injection, it's only used once in the inject() method, to initialise the values in the delegates, and then released.

Not entirely sure why your model is returning HomeModule type though. Is that a typo or am I misunderstanding your example?

@sindrenm
Copy link
Author

Hey!

Not entirely sure why your model is returning HomeModule type though.

Yeah, sorry, that was a typo. I've edited the original post. Sorry about that. 😛

Regarding the reference issue, you're absolutely right. I don't know how I misunderstood that before, but looking at the code now it's pretty clear. I did have a memory leak, however, and I traced it back to being kept by the lastKey (or possibly lastValue, can't quite recall), but then that is probably all my fault. All you're doing with the module is looping through delegates and initializing them.

I'll close this and fix up my code. 😛

Thanks for your reply. 😀

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

No branches or pull requests

2 participants