Dagger: Android injection leaks objects

[bubenheimer]

The current design for Android injection from 2.10RC2 leaks Android framework objects. This is due to the builder lookup Maps retaining references to AndroidInjector.Factory objects after their initial creation via a Provider, and the AndroidInjector.Factory (Builder) objects retaining references to Android framework objects passed in via seedInstance().

The result is that Android framework objects are not garbage-collected until the next time that an object of the same class is injected, which may never happen, but in any case is far too late.

[ronshapiro]

The providers create a new factory/builder each time, and then it is thrown away right after injection occurs. Are you scoping your builder somewhere?

[bubenheimer]

Thanks for the heads-up! I was scoping the Map entries to make them available at just the right level. I guess that was a bad idea.

[ronshapiro]

Mind showing what you were doing? It might make sense to have a warning/error in the processor for this

[bubenheimer]

In my app I use several scope levels like @Singleton (application-wide scope), @ActivityScope, and @FragmentScope. I scope most things that I provide in Modules, so it seemed natural to also scope multibound map entries, as their lifetime & visibility would need to follow the parent scope's lifetime & visibility. I now realize that I get this automatically from putting them into the right component/subcomponent, without specifying scope.

For example, what I did:

@Module
abstract class InitFragmentMapModule {
    @Binds
    @ActivityScope
    @IntoMap
    @FragmentKey(InitFragment.class)
    abstract AndroidInjector.Factory<? extends Fragment>
        bindFactory(InitFragmentComponent.Builder builder);
}

Somewhat related - could the user guide be more explicit about the lifetime of unscoped Dagger objects? I find it a little vague on that topic right now, and more explicitly contrasting it with scoped object behavior would remove lingering doubts. The vagueness led me to overuse scope. In particular add information about:

1. When are new unscoped objects created via @Inject constructors?
2. When do unscoped @Provides methods get called?
3. What changes when I add scope to @IntoMap/@IntoSet?

[JakeWharton]

The answer to 1 and 2 are the same:

Any time that type is requested by any of the ways you can get an instance
from Dagger (while creating an object via constructor, as a dependency of
another provides method, as a method on the component).

Or put another way, when a binding is unscoped an instance is created each
time something else in the graph asks for an instance.

On Mon, Mar 6, 2017, 1:51 AM Uli Bubenheimer [e-mail]
wrote:

In my app I use several scope levels like @Singleton
https://github.com/Singleton (application-wide scope), @ActivityScope,
and @FragmentScope. I scope most things that I provide in Modules, so it
seemed natural to also scope multibound map entries, as their lifetime &
visibility would need to follow the parent scope's lifetime & visibility. I
now realize that I get this automatically from putting them into the right
component/subcomponent, without specifying scope.

For example, what I did:

@Module
abstract class InitFragmentMapModule {
@Binds
@ActivityScope
@IntoMap
@FragmentKey(InitFragment.class)
abstract AndroidInjector.Factory
bindFactory(InitFragmentComponent.Builder builder);
}

Somewhat related - could the user guide be more explicit about the
lifetime of unscoped Dagger objects? I find it a little vague on that topic
right now, and more explicitly contrasting it with scoped object behavior
would remove lingering doubts. The vagueness led me to overuse scope. In
particular add information about:

1. When are new unscoped objects created via @Inject
   https://github.com/Inject constructors?
2. When do unscoped @Provides https://github.com/Provides methods
   get called?
3. What changes when I add scope to @IntoMap/@IntoSet?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/dagger/issues/612#issuecomment-284316367, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEEERZ-Er-W89hgOreVHiZt4wxX-ARRks5ri60LgaJpZM4MTTUi
.

[bubenheimer]

Thanks @JakeWharton! I did remove my lingering doubts eventually via sample code, but it is always good to receive explicit confirmation. My thought was that a short paragraph about this in the manual might help others, like in the section about Singleton and scope.

[ronshapiro]

The answer to #3 is that if you add scope to the @Binds @IntoSet/@IntoMap method, is that the usage of that binding is scoped when it is used in the multibinding, even if the binding itself isn't scoped in other places.

Consider:

@Provides String s() {
  return "hello" + someCounter++;
}

@Binds @IntoSet @Singleton String bindStringToSet(String s);
@Binds @IntoSet String bindOtherStringToSet(String s);

When you request that set, the first time it will have ["hello0", "hello1"], the next time ["hello0", "hello2"], then ["hello0", "hello3"] etc. I.e. the hello0 is scoped, even though the @Provides method isn't.

Note that this is only partially correct, since whichever one of these gets called first is not specified, so it could be ["hello0", "hello1"] the first time, then ["hello0", "hello1"], then ["hello2", "hello1"], then ["hello3", "hello1"], but hopefully that shows the point.

[bubenheimer]

That just gave me a good laugh on a Monday morning. Makes me wonder if there is any valid use case for this feature.

[ronshapiro]

It's a trivial example. Also, modifying state in a provides method is an antipattern, but it helps show what actually happens