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

Dagger Application injection NPE #748

Closed
luispereira opened this issue May 23, 2017 · 31 comments
Closed

Dagger Application injection NPE #748

luispereira opened this issue May 23, 2017 · 31 comments

Comments

@luispereira
Copy link

luispereira commented May 23, 2017

I'm trying to inject some objects on application, in order to access them whenever I want.
So, on my application I'm doing something like this:

private static AppComponent mApplicationComponent;

@Override
    public void onCreate() {
        …
       mApplicationComponent = DaggerAppComponent.builder()
                .appModule(new AppModule(this, new ModuleSpref(this))
                .build();
   }

 public static AppComponent getInstance() {
        return mApplicationComponent;
    }

However, according to google crash log of my production app, the application has a NullPointException when trying to access anything from getInstance(). On my application this happens on a service which is only called via a receiver. Additionally, this also happens on another receiver.

 MyApplication.getInstance().context(); //Null Point Exception :(

Here is my ApplicationComponent:

@Component(modules = AppModule.class)
@Singleton
public interface AppComponent {
    void inject(Context context);

    //Exposed to sub-graphs.
    Context context();
    ModuleSpref getSpref();
}

I tried many times to replicate, but so far without success, despite the google crash saying otherwise.
Am I doing something wrong here?
As far as I know, for services/receivers to be initialised in android, the application must first be initialised, so getInstance() should not return null right?

@ronshapiro
Copy link

Take a look at https://google.github.io/dagger/api/latest/dagger/android/DaggerApplication.html, which handles this scenario where sometimes a contentprovider.oncreate() is called before the application.oncreate()

@luispereira
Copy link
Author

I can't see the link (404) for me.

@luispereira
Copy link
Author

Also, getInstance() is only called on onReceiver() of broadcast or on startCommand() of service

@tbroyer
Copy link

tbroyer commented May 24, 2017

Ron probably got his markdown mixed up and meant DaggerApplication

@luispereira
Copy link
Author

luispereira commented May 24, 2017

Thanks, so if I undestrood correctly, injecting my objects on a DaggerApplication class will prevent this injection before application.oncreate() right?
Also, is there any exemple with DaggerApplication?

@luispereira
Copy link
Author

luispereira commented May 26, 2017

I'm actually trying to do something about this,

public MyApplication extends DaggerApplication implements AndroidInjector<MyApplication>

When I notice that AndroidInjector only has, broadcast, service, activity, fragment or contentprovider.

So, how should I proceed when overriding the method applicationInjector on application?

  @Override
    protected AndroidInjector<? extends DaggerApplication> applicationInjector() {
        return this;
    }
    @Override
    public void inject(MyApplication myApplication) {
        //this does not exist, so how can I create a AndroidInjector for application? or am I missing somthing?
        AndroidInjector<MyApplication> applicationInjector = ((HasApplicatiomInjector) myApplication).applicationInjector();
        applicationInjector.inject(this);
    }

Since I'm only injecting this to access it from everywhere, activities, services and broadcasts.

@matpag
Copy link

matpag commented Jun 11, 2017

I'm stucked at the same point, i'm pretty new to Dagger 2 and there is no documentation on how to use the new DaggerApplication class (every tutorial online is somehow not updated)

@luispereira
Copy link
Author

luispereira commented Jun 26, 2017

As a side note, this NPE on injected instance happens a lot on services from startCommand() and on samsung devices such as Galaxy S8, S7 and S6

    @Override
    public int onStartCommand(Intent intent, int flags, int startId) {
        MyApplication.getInstance().context(); //just to text if no npe is given
       
        (...) 

        return START_REDELIVER_INTENT;
    }

@ronshapiro
Copy link

Github isn't the best forum for this - please use StackOverflow with the dagger-2 tag instead, as that is a better forum for questions.

@juliahexen
Copy link

@luispereira Did you find any solution for your problem? Same here :/

@luispereira
Copy link
Author

luispereira commented Oct 11, 2017

Actually I found a work around for this which implies something like this on your application class:

public class MyApplication extends Application {
    private static MyApplication sInstance;
    private AppComponent applicationComponent;

    public MyApplication() {
        sInstance = this;
    } 

    public static AppComponent getInstance() {
        return sInstance.getAppComponent();
    }

    private AppComponent getAppComponent() {
        if (applicationComponent == null) {
            //initialise the dagger stuff here
        }
        return applicationComponent;
    }
}

Basically, if by any instance, the dagger app component builder returns null, it will force to initialise again. I know this is far from a perfect solution, but it works.

PS: I had multiple reports of different projects which the application was started after services/brodcasts and even activities on Samsung devices. I'm still investigating the issue, but I seen that Samsung starts a content provider for every application, maybe triggering this kind of behaviour. Although, with the above solution this kind of NPE no longer happened

@juliahexen
Copy link

Thank you very much, I will try it :)

@dnsknr
Copy link

dnsknr commented Oct 16, 2017

I am facing this issue as well and I have no idea how to work around it.

Caused by: java.lang.RuntimeException: 
  at dagger.android.AndroidInjection.inject (AndroidInjection.java:48)
  at dagger.android.support.DaggerAppCompatActivity.onCreate (DaggerAppCompatActivity.java:43)
  at com.package.MainActivity.onCreate (MainActivity.java:83)
  at android.app.Activity.performCreate (Activity.java:6912)
  at android.app.Instrumentation.callActivityOnCreate (Instrumentation.java:1126)
  at android.app.ActivityThread.performLaunchActivity (ActivityThread.java:2900)

This happens only on Android 7.0, mainly on Samsung devices, some Huawai and Motorola devices and a few Xperia devices. I am unable to reproduce it since I do not have any of the affected devices at hand.

It seems this is caused by the application not (yet) being set for the activity, meaning activity's onCreate is called before the activity is actually attached to the application.

Here the affected Dagger code:

protected void onCreate(@Nullable Bundle savedInstanceState) {
AndroidInjection.inject(this);
super.onCreate(savedInstanceState);
}

public static void inject(Activity activity) {
checkNotNull(activity, "activity");
Application application = activity.getApplication();
if (!(application instanceof HasActivityInjector)) {
throw new RuntimeException(
String.format(
"%s does not implement %s",
application.getClass().getCanonicalName(),
HasActivityInjector.class.getCanonicalName()));
}

activity.getApplication() returns null, and so the RuntimeException is thrown.

Anyone any suggestions?

@Zhuinden
Copy link

Zhuinden commented Oct 17, 2017

Oh this is probably the same problem as what Mortar fixes with

Application app = activity.getApplication();
if(app == null) {
     app = (Application)activity.getApplicationContext();
}

@ronshapiro ronshapiro reopened this Oct 17, 2017
@ronshapiro
Copy link

@Zhuinden is this what you're referring too: square/mortar@bf74e3e?

Is there any reason to not just call (Application) activity.getApplicationContext() here then and not worry about the null-check?

/cc @rjrjr @loganj

@JakeWharton
Copy link

JakeWharton commented Oct 17, 2017 via email

@dnsknr
Copy link

dnsknr commented Oct 17, 2017

Hi everyone,

I rolled out an update a few hours ago with the change @Zhuinden proposed based on Dagger's DaggerAppCompatActivity and the AndroidInjection class:

public abstract class MyDaggerAppCompatActivity extends AppCompatActivity implements HasFragmentInjector, HasSupportFragmentInjector {

    @Inject DispatchingAndroidInjector<Fragment> supportFragmentInjector;
    @Inject DispatchingAndroidInjector<android.app.Fragment> frameworkFragmentInjector;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        inject();
        super.onCreate(savedInstanceState);
    }

    @Override
    public AndroidInjector<Fragment> supportFragmentInjector() {
        return supportFragmentInjector;
    }

    @Override
    public AndroidInjector<android.app.Fragment> fragmentInjector() {
        return frameworkFragmentInjector;
    }

    private void inject() {
        Application application = getApplication();
        if(application == null) {
            application = (Application) getApplicationContext();
        }

        if (!(application instanceof HasActivityInjector)) {
            throw new RuntimeException(String.format("%s does not implement %s", application.getClass().getCanonicalName(), HasActivityInjector.class.getCanonicalName()));
        }

        AndroidInjector<Activity> activityInjector = ((HasActivityInjector) application).activityInjector();
        checkNotNull(activityInjector, "%s.activityInjector() returned null", application.getClass().getCanonicalName());

        activityInjector.inject(this);
    }

}

Still getting the RuntimeException, now thrown by my code above. So looks like that either there is an application but the wrong one or even the application context is null. Cannot imagine how any of both scenarios would work actually...

@dnsknr
Copy link

dnsknr commented Oct 23, 2017

I tested using getApplicationContext() instead of getApplication() in all Dagger 2 components I am using (DaggerAppCompatActivity, DaggerIntentService, DaggerFragment) and I can confirm that it does not make any difference. This version is now rolled out to over 300k users and the exception rate is no different to the original version based on unchanged Dagger 2.12 code.

See also https://stackoverflow.com/questions/46784685/runtimeexception-with-dagger-2-on-android-7-0-and-samsung-devices where I posted some code where I tried to get more understanding of this error without any luck.

@tbroyer
Copy link

tbroyer commented Oct 23, 2017

See also #891

@dnsknr
Copy link

dnsknr commented Oct 25, 2017

I solved the issue for myself by not using anymore the DaggerApplication but an Application independent solution described here: https://stackoverflow.com/a/46925736/8788319

Not sure if this is an option for Dagger 2 but I did not find another way to resolve the crashes under Android 7.0 otherwise.

@abhiroopnray
Copy link

Hope you had added application class name in the Android Manifest file. Without it, the app will work but the dagger component will be null.

@ronshapiro
Copy link

I'm still pretty dumbfounded at what is happening here. I wish I had a better suggestion at what we should do.

@pablobaxter
Copy link

pablobaxter commented Apr 9, 2020

One interesting thing to point out is that Android doesn't always run any of the ContentProvider classes or the Application class listed in the manifest. https://developer.android.com/guide/topics/data/autobackup#ImplementingBackupAgent

Specifically...
image

I've tested to see if Services and BroadcastReceivers still ran, and they do. This was causing multiple issues for me with Fabric, Firebase, and Dagger. I figured I'd share my findings here as I referenced this issue to try to fix my Dagger problem.

I'm in the process of disabling this auto backup (setting allowBackup="false" in the AndroidManifest) for the app I work on to validate my fix. I'll post my findings if it fixes my Dagger issue.

@camsteffen
Copy link

@pablobaxter Any update?

@pablobaxter
Copy link

@pablobaxter Any update?

@camsteffen Thank you for reminding me! Also sorry... I never did update my result on this thread as promised.

So disabling the backup worked for me. I am no longer seeing my Dagger crash, which in my case was a ClassCastException due to getApplication() not being an instance of DaggerApp, even though I extend it. This also solved one of the other issues I had, which would only ever happen if my extended Application class never ran.

I filed a ticket about this with Firebase (firebase/firebase-android-sdk#1456), as this was the main library starting Services and BroadcastReceivers while the app was in this restricted mode. However, this is most likely an Android OS issue. It looks like whenever the app crashes in this restricted mode (in my case due to Firebase starting up), the next launch app (user initiated or otherwise) restarts the app in this restricted mode, which caused a crash, and so on.

Hope this helps!

@camsteffen
Copy link

@pablobaxter Thanks for the update! It looks like your firebase bug report is attracting some more needed attention so I will follow that, even though I don't use firebase myself. I wonder if disabling backup would prevent this issue in all cases.

@Chang-Eric
Copy link
Member

Closing this as I don't think there is much that can be done on the Dagger side. This feels like an Android bug that you can ever be called without the Application specified in the manifest.

@camsteffen
Copy link

Given the Android docs quoted and the SO solution mentioned, would it make sense for Dagger to provide a solution to work without overriding Application?

@pablobaxter
Copy link

As an FYI, I did file a bug with the Android team regarding the backup agent. https://issuetracker.google.com/u/1/issues/160946170

@Chang-Eric
Copy link
Member

@camsteffen if I understand the SO solution, it is just moving the component to a static variable? This would work for the production app, but keeping your DI graph in static state tends to cause problems for things like tests.

@Zhuinden
Copy link

@camsteffen if I understand the SO solution, it is just moving the component to a static variable? This would work for the production app, but keeping your DI graph in static state tends to cause problems for things like tests.

That's never bothered anyone using Koin, which works exactly the same way.

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

No branches or pull requests