google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.41k stars 2.01k forks source link

Null reference used for synchronization with Hilt #1904

Open NitroG42 opened 4 years ago

NitroG42 commented 4 years ago

Hello, I was happy trying Hilt unfortunatelly I have an issue on a new project I began 2 weeks ago. Simple project with activity/fragment/viewmodel navigation etc...

However I get a crash on the first fragment I display, I first tough it came from the viewmodel but if I inject any variable the crash occurs. If I remove any thing that would require injection on the fragment, no crash:

2020-06-11 17:38:00.980 22646-22646/com.myapp.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.myapp.debug, PID: 22646
    java.lang.NullPointerException: Null reference used for synchronization (monitor-enter)
        at com.myapp.ui.login.Hilt_LoginFragment.componentManager(Hilt_LoginFragment.java:88)
        at com.myapp.ui.login.Hilt_LoginFragment.generatedComponent(Hilt_LoginFragment.java:79)
        at com.myapp.ui.login.Hilt_LoginFragment.inject(Hilt_LoginFragment.java:98)
        at com.myapp.ui.login.Hilt_LoginFragment.initializeComponentContext(Hilt_LoginFragment.java:62)
        at com.myapp.ui.login.Hilt_LoginFragment.onAttach(Hilt_LoginFragment.java:54)
        at androidx.fragment.app.Fragment.onAttach(Fragment.java:1758)
        at com.myapp.ui.login.Hilt_LoginFragment.onAttach(Hilt_LoginFragment.java:44)
        at androidx.fragment.app.Fragment.performAttach(Fragment.java:2853)

Here is the generated fragment and just after the crashing line:

@Generated("dagger.hilt.android.processor.internal.androidentrypoint.FragmentGenerator")
public abstract class Hilt_LoginFragment extends AbstractFragment implements GeneratedComponentManager<Object> {
  private ContextWrapper componentContext;

  private volatile FragmentComponentManager componentManager;

  private final Object componentManagerLock = new Object();

  Hilt_LoginFragment(@LayoutRes int layoutId) {
    super(layoutId);
  }

  Hilt_LoginFragment() {
    super();
  }

  @Override
  @CallSuper
  public void onAttach(Context context) {
    super.onAttach(context);
    initializeComponentContext();
  }

  @Override
  @CallSuper
  @MainThread
  public void onAttach(Activity activity) {
    super.onAttach(activity);
    Preconditions.checkState(componentContext == null || FragmentComponentManager.findActivity(componentContext) == activity, "onAttach called multiple times with different Context! Hilt Fragments should not be retained.");
    initializeComponentContext();
  }

  private void initializeComponentContext() {
    // Only inject on the first call to onAttach.
    if (componentContext == null) {
      // Note: The LayoutInflater provided by this componentContext may be different from super Fragment's because we getting it from base context instead of cloning from the super Fragment's LayoutInflater.
      componentContext = FragmentComponentManager.createContextWrapper(super.getContext(), this);
      inject();
    }
  }

  @Override
  public Context getContext() {
    return componentContext;
  }

  @Override
  public LayoutInflater onGetLayoutInflater(Bundle savedInstanceState) {
    LayoutInflater inflater = super.onGetLayoutInflater(savedInstanceState);
    return LayoutInflater.from(FragmentComponentManager.createContextWrapper(inflater, this));
  }

  @Override
  public final Object generatedComponent() {
    return componentManager().generatedComponent();
  }

  protected FragmentComponentManager createComponentManager() {
    return new FragmentComponentManager(this);
  }

  protected final FragmentComponentManager componentManager() {
    if (componentManager == null) {
      synchronized (componentManagerLock) {
        if (componentManager == null) {
          componentManager = createComponentManager();
        }
      }
    }
    return componentManager;
  }

  protected void inject() {
    ((LoginFragment_GeneratedInjector) generatedComponent()).injectLoginFragment(UnsafeCasts.<LoginFragment>unsafeCast(this));
  }

  @Override
  public ViewModelProvider.Factory getDefaultViewModelProviderFactory() {
    ViewModelProvider.Factory factory = DefaultViewModelFactories.getFragmentFactory(this);
    if (factory != null) {
      return factory;
    }
    return super.getDefaultViewModelProviderFactory();
  }
}
  protected final FragmentComponentManager componentManager() {
    if (componentManager == null) {
      synchronized (componentManagerLock) {  //componentManagerLock IS NULL
        if (componentManager == null) {
          componentManager = createComponentManager();
        }
      }
    }
    return componentManager;
  }

I tried with the debugger and the componentManagerLock IS null when we try to synchronise! Which makes no sense to me... I suppose it can be a bytecode transformation issue, so I tried with android gradle plugin 4.0.0 and 4.1.0-beta01/gradle 6.5 but same issue. I'm on windows 10 x64 and running the project on an emulator.

I tried on a sample project but the issue doesn't occurs, and I can't share code of my current project (it's for a client). If I can provide any help tell me, I'm too tired right now but I'll work on what triggers this/sample later.

Chang-Eric commented 4 years ago

Hm, this seems impossible... The only thing I can think of is if your base class constructor call is somehow directly triggering performAttach before the initialization of that variable finishes and so you're trying to inject things before your constructor finishes. Is there anything interesting in AbstractFragment going on?

If you suspect the bytecode transformation, you could remove the plugin and directly extend the generated base class as described here and see if that does anything. https://dagger.dev/hilt/gradle-setup#why-use-the-plugin

NitroG42 commented 4 years ago

I can put the classes here :

AbstractFragment.kt (I don't have any methods called here on fragment initialization I think)

abstract class AbstractFragment(@LayoutRes layoutId: Int = 0) : Fragment(layoutId) {
    protected var snackbar: Snackbar? = null

    protected fun dismissSnackbar() = snackbar?.dismiss()

    fun displayErrorSnackbar(
        message: String,
        configuration: Snackbar.() -> Unit = { addOkButton() }
    ) =
        displaySnackbar(message, Snackbar.LENGTH_INDEFINITE).apply(configuration).displayError()

    fun displayErrorSnackbar(
        @StringRes message: Int,
        configuration: Snackbar.() -> Unit = { addOkButton() }
    ) =
        displayErrorSnackbar(getString(message), configuration)

    fun displayErrorSnackbar(throwable: Throwable?) = displayErrorSnackbar(throwable.getErrorMessage(requireContext()))

    fun displaySnackbar(
        @StringRes message: Int,
        @BaseTransientBottomBar.Duration length: Int = Snackbar.LENGTH_INDEFINITE,
        configuration: Snackbar.() -> Unit = {}
    ) = displaySnackbar(message, length).apply(configuration).display()

    private fun displaySnackbar(
        @StringRes message: Int,
        @BaseTransientBottomBar.Duration length: Int = Snackbar.LENGTH_INDEFINITE
    ) = Snackbar.make(requireView(), message, length).also {
        dismissSnackbar()
        snackbar = it
    }

    private fun displaySnackbar(
        message: String,
        @BaseTransientBottomBar.Duration length: Int = Snackbar.LENGTH_INDEFINITE
    ) = Snackbar.make(requireView(), message, length).also {
        dismissSnackbar()
        snackbar = it
    }

    protected fun Toolbar.init() {
        inflateMenu(R.menu.workflows_menu)
        menu.findItem(R.id.menuLogout).asLogoutView {
            logoutButton.setOnClickListener {
                val dialogLogout = LogoutDialog()
                childFragmentManager.beginTransaction()
                    .add(dialogLogout, "dialog_logout")
                    .commitAllowingStateLoss()
            }
        }
    }

    protected fun MenuItem.asLogoutView(action: MenuLogoutBinding.() -> Unit) = action(MenuLogoutBinding.bind(actionView))

    protected fun Toolbar.setConnectivity(connected: Boolean) {
        val text: String
        val icon: Int
        if (connected) {
            text = getString(R.string.workflows_online)
            icon = R.drawable.icon_online_resgen
        } else {
            text = getString(R.string.workflows_offline)
            icon = R.drawable.icon_offline_resgen
        }
        menu.findItem(R.id.menuConnectivity).actionView?.let {
            (it as TextView).text = text
            it.setCompoundDrawablesRelativeWithIntrinsicBounds(0, 0, icon, 0)
        }
    }

    protected fun Toolbar.observeConnectivity(connectivityLD: LiveData<Boolean>) {
        connectivityLD.observe(viewLifecycleOwner) {
            setConnectivity(it)
        }
    }

}

LoginFragment.kt:

@AndroidEntryPoint
class LoginFragment : AbstractFragment() {
    private var binding: FragmentLoginBinding? = null

    @Inject
    lateinit var url: HttpUrl

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        return FragmentLoginBinding.inflate(inflater).also { binding = it }.root
    }

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        binding?.apply {
            Timber.d("$url")
        }
    }

    override fun onDestroyView() {
        super.onDestroyView()
        binding = null
    }
}

And the module that provide the injected string:

@Module
@InstallIn(ApplicationComponent::class)
object ConstantModule {

    @Provides
    fun url(@ApplicationContext context: Context): HttpUrl = context.getString(R.string.base_url).toHttpUrl()

}

I will try without the plugin and say the result quickly

NitroG42 commented 4 years ago

Removing the plugins and every others AndroidEntryPoint (except Login), it worked! I put the plugin back modifying just the Application class, the MainActivity and the LoginFragment (for AndroidEntryPoint) => Crash So it seems like a plugin/byte code issue ? I'll try to remove isolate the issue now

NitroG42 commented 4 years ago

I found the issue, and I just didn't put the right line on my first try of a sample (I'm sad I lost 2 hours). The issue comes from my AbstractFragment having this constructor:

abstract class AbstractFragment(@LayoutRes layoutId: Int = 0) : Fragment(layoutId)

Using

abstract class AbstractFragment: Fragment()

or

abstract class AbstractFragment: Fragment(0)

(This one was what I tried in my sample... I should have pasted the whole line!!!)

Make the issue appears instantly. Bonus: I now have an easy fix as we were not using this argument anyway. I'll post the sample repo in an edit in a few seconds.

NitroG42 commented 4 years ago

Here's the sample: https://github.com/NitroG42/HiltAbstractFragmentIssue

Chang-Eric commented 4 years ago

Thanks for all of the details! We'll investigate why this is happening and hopefully fix this soon.

SteinerOk commented 4 years ago

Have same issue, but in activity if use constructor with @LayoutRes layoutId: Int = 0

danysantiago commented 4 years ago

The issues occurs when the immediate superclass of an @AndroidEntryPoint annotated class has a constructor with default parameters.

The root problem is that the generated Hilt_ class is in Java and the mechanism for which methods with default values are called is lost during the bytecode transformation, causing the unexpected behaviour.

You can workaround by using the 'long-form' of @AndroidEntryPoint, i.e.:

@AndroidEntryPoint(AbstractFragment::class)
MyFragment : Hilt_MyFragment() { }

Which will follow the Java->Kotlin interop path and get the expected results. Additionally you can use @JvmOverloads in case you have multiple default params.

The solution we have in mind so far is a bit complicated because we might need to rely on the synthetic method bitmask and kotlin/jvm/internal/DefaultConstructorMarker interpretation, which is a bit scary. In the meantime we can detect this scenario and throw a more clear error with the workaround.

kaneoriley commented 4 years ago

In case anyone else has this issue, the suggested workaround of using the 'long-form' didn't work for me, however making an intermediate class with no constructor parameters did work:

// Workaround for https://github.com/google/dagger/issues/1904
abstract class BaseMainActivity : BaseActivity<ActivityMainBinding>(ActivityMainBinding::class)

@AndroidEntryPoint
class MainActivity : BaseMainActivity() {
    // rest of activity, field injection working as expected.
}

Original code:

@AndroidEntryPoint
class MainActivity : BaseActivity<ActivityMainBinding>(ActivityMainBinding::class) {
    // fails with 'Null reference used for synchronization (monitor-enter) error'
}
yschimke commented 8 months ago

I was getting this error. In my case my super class had optional params. Specifying them all instead of defaulting fixed the issue.

abstract class BaseWatchFaceService(
    private val watchFaceType: Int = WatchFaceType.ANALOG,
    complicationSlotIds: Set<Int>
) : WatchFaceService() {

@AndroidEntryPoint()
class MyWatchFaceService :
    BaseWatchFaceService(complicationSlotIds = setOf(0), watchFaceType = WatchFaceType.DIGITAL) {