bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.61k stars 6.12k forks source link

IllegalStateException: You cannot call Glide.get() in registerComponents() = bad error message #2780

Closed pandasys closed 6 years ago

pandasys commented 6 years ago

Glide Version: glide:4.3.1

Integration libraries: okhttp3-integration:4.3.1

Device/Android Version: Various, reported from field. Stacktrace comes from Samsung Galaxy S5 (klteatt) Android 6.0

Issue details / Repro steps / Use case background: The exception text is misleading as the problem is not occuring in my AppGlideModule implementation.

This is difficult to reproduce as it appears to be a timing issue. I'm getting several reports of crashes and the location varies. Possibly related to Kotlin coroutines, but this did not occur on Glide pre 4.0. The stacktrace indicates this is not inside a Kotlin coroutine.

I've tried adding a "dummy" Glide request of Uri.Empty at the beginning of onCreate() to init Glide, but this hasn't helped.

    // force glide initialization
    try {
      if (!glideInitialized.getAndSet(true)) {
        debug { LOG.log(LogLevel.ERROR, "Initialize Glide") }
        Glide.with(this).load(Uri.EMPTY).submit(10, 10)
      }
    } catch (e: Exception) {
      debug { LOG.caught(LogLevel.ERROR, e) }
    }

Can I force Glide init in some other manner? While I appreciate lazy loading resources I may never use, my app will use Glide and my loaders when my first activity starts and I'm looking for a workaround. The code snippet is related to the included stack trace, however the problem occurs at many different Glide.with() invocations randomly. After a Glide.with() executes without error, the issue does not reoccur.

  override fun onBindViewHolder(holder: ViewHolder, position: Int) {
    val image = imageList[position]
    val uri = Uri.parse(image.text)
    val options = RequestOptions()
        .diskCacheStrategy(DiskCacheStrategy.DATA)
    if (LastFm.IMAGE_SIZE_MEGA == image.size) {
      options.override(OVERRIDE_SIZE, OVERRIDE_SIZE)
          .fitCenter()
    }

    Glide.with(activity)
        .asBitmap()
        .load(uri)
        .apply(options)
        .listener(object : RequestListener<Bitmap> {
          override fun onResourceReady(p0: Bitmap?, p1: Any?, p2: Target<Bitmap>?, p3: DataSource?, p4: Boolean): Boolean {
            holder.progressBar.visibility = View.GONE
            return false
          }

          override fun onLoadFailed(p0: GlideException?, p1: Any?, p2: Target<Bitmap>?, p3: Boolean): Boolean {
            holder.progressBar.visibility = View.GONE
            return false
          }
        })
        .into(holder.albumArtView)

Layout XML:

<?xml version="1.0" encoding="utf-8"?>

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
              xmlns:app="http://schemas.android.com/apk/res-auto"
              xmlns:tools="http://schemas.android.com/tools"
              android:layout_width="match_parent"
              android:layout_height="match_parent"
              android:padding="8dp"
    >

    <android.support.v7.widget.CardView
        android:id="@+id/album_art_card"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:background="?attr/ap_secondaryBackground"
        >

        <RelativeLayout
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            >

            <fr.castorflex.android.circularprogressbar.CircularProgressBar
                android:id="@+id/last_art_card_progress"
                android:layout_width="40dp"
                android:layout_height="40dp"
                android:layout_centerInParent="true"
                android:indeterminate="true"
                app:cpb_color="?attr/ap_secondaryBackground"
                app:cpb_colors="@array/gplus_colors"
                app:cpb_rotation_speed="1.0"
                app:cpb_sweep_speed="1.0"
                app:cpb_stroke_width="4dp"
                app:cpb_min_sweep_angle="10"
                app:cpb_max_sweep_angle="300"
                />

            <ImageView
                android:id="@+id/album_art_image"
                android:layout_width="156dp"
                android:layout_height="156dp"
                android:layout_alignParentTop="true"
                android:layout_marginBottom="6dp"
                android:layout_centerHorizontal="true"
                tools:ignore="ContentDescription"
                />

            <TextView
                android:id="@+id/album_art_description"
                android:layout_width="match_parent"
                android:layout_height="wrap_content"
                android:layout_below="@+id/album_art_image"
                android:gravity="center_horizontal"
                android:textSize="18sp"
                />

        </RelativeLayout>

    </android.support.v7.widget.CardView>

</LinearLayout>

Stack trace / LogCat:

java.lang.IllegalStateException: 
   at com.bumptech.glide.Glide.checkAndInitializeGlide (Glide.java:172)
   at com.bumptech.glide.Glide.get (Glide.java:160)
   at com.bumptech.glide.Glide.getRetriever (Glide.java:583)
   at com.bumptech.glide.Glide.with (Glide.java:620)
   at com.ealva.alvaplayer.ui.image.LastFmImageAdapter.onBindViewHolder (LastFmImageAdapter.java:83)
   at com.ealva.alvaplayer.ui.image.LastFmImageAdapter.onBindViewHolder (LastFmImageAdapter.java:48)
   at android.support.v7.widget.RecyclerView$Adapter.onBindViewHolder (RecyclerView.java:6508)
   at android.support.v7.widget.RecyclerView$Adapter.bindViewHolder (RecyclerView.java:6541)
   at android.support.v7.widget.RecyclerView$Recycler.tryBindViewHolderByDeadline (RecyclerView.java:5484)
   at android.support.v7.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline (RecyclerView.java:5750)
   at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition (RecyclerView.java:5589)
   at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition (RecyclerView.java:5585)
   at android.support.v7.widget.LinearLayoutManager$LayoutState.next (LinearLayoutManager.java:2231)
   at android.support.v7.widget.GridLayoutManager.layoutChunk (GridLayoutManager.java:556)
   at android.support.v7.widget.LinearLayoutManager.fill (LinearLayoutManager.java:1518)
   at android.support.v7.widget.LinearLayoutManager.onLayoutChildren (LinearLayoutManager.java:610)
   at android.support.v7.widget.GridLayoutManager.onLayoutChildren (GridLayoutManager.java:170)
   at android.support.v7.widget.RecyclerView.dispatchLayoutStep2 (RecyclerView.java:3719)
   at android.support.v7.widget.RecyclerView.dispatchLayout (RecyclerView.java:3436)
   at android.support.v7.widget.RecyclerView.onLayout (RecyclerView.java:3988)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.RelativeLayout.onLayout (RelativeLayout.java:1080)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.support.design.widget.CoordinatorLayout.layoutChild (CoordinatorLayout.java:1171)
   at android.support.design.widget.CoordinatorLayout.onLayoutChild (CoordinatorLayout.java:856)
   at android.support.design.widget.CoordinatorLayout.onLayout (CoordinatorLayout.java:875)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.support.v4.widget.DrawerLayout.onLayout (DrawerLayout.java:1172)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.FrameLayout.layoutChildren (FrameLayout.java:344)
   at android.widget.FrameLayout.onLayout (FrameLayout.java:281)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.LinearLayout.setChildFrame (LinearLayout.java:1742)
   at android.widget.LinearLayout.layoutVertical (LinearLayout.java:1585)
   at android.widget.LinearLayout.onLayout (LinearLayout.java:1494)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.FrameLayout.layoutChildren (FrameLayout.java:344)
   at android.widget.FrameLayout.onLayout (FrameLayout.java:281)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.LinearLayout.setChildFrame (LinearLayout.java:1742)
   at android.widget.LinearLayout.layoutVertical (LinearLayout.java:1585)
   at android.widget.LinearLayout.onLayout (LinearLayout.java:1494)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.widget.FrameLayout.layoutChildren (FrameLayout.java:344)
   at android.widget.FrameLayout.onLayout (FrameLayout.java:281)
   at com.android.internal.policy.PhoneWindow$DecorView.onLayout (PhoneWindow.java:3193)
   at android.view.View.layout (View.java:17945)
   at android.view.ViewGroup.layout (ViewGroup.java:5814)
   at android.view.ViewRootImpl.performLayout (ViewRootImpl.java:2717)
   at android.view.ViewRootImpl.performTraversals (ViewRootImpl.java:2418)
   at android.view.ViewRootImpl.doTraversal (ViewRootImpl.java:1488)
   at android.view.ViewRootImpl$TraversalRunnable.run (ViewRootImpl.java:7451)
   at android.view.Choreographer$CallbackRecord.run (Choreographer.java:920)
   at android.view.Choreographer.doCallbacks (Choreographer.java:695)
   at android.view.Choreographer.doFrame (Choreographer.java:631)
   at android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:906)
   at android.os.Handler.handleCallback (Handler.java:739)
   at android.os.Handler.dispatchMessage (Handler.java:95)
   at android.os.Looper.loop (Looper.java:158)
   at android.app.ActivityThread.main (ActivityThread.java:7224)
   at java.lang.reflect.Method.invoke (Method.java)
   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:1230)
   at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1120)
sjudd commented 6 years ago

I don't see any instances of these in any of the applications I work on.

Initialization happens under a lock here: https://github.com/bumptech/glide/blob/cc4bb160c555e90c354f5c4c9c2722756709232f/library/src/main/java/com/bumptech/glide/Glide.java#L164

The check itself happens when that initialization begins here: https://github.com/bumptech/glide/blob/cc4bb160c555e90c354f5c4c9c2722756709232f/library/src/main/java/com/bumptech/glide/Glide.java#L176

Prior to Glide 4.0 you were probably silently ending up with multiple Glide singleton objects and corresponding caches. In Glide 4.0 there's now an assertion.

Just Glide.get(context)is sufficient to initialize Glide.

It's totally possible there's a logic error here, but I don't see anything obvious. I'd also expect to see instances of the error in feedback reports in the applications I do work on if it were a general issue. I'm open to ideas. Right now it seems like two different threads are able to acquire the class lock on Glide at once, which doesn't seem like it should be possible.

pandasys commented 6 years ago

I had visually inspected the areas you mention and I don't see how it's possible either, especially given that each exception originates on the UI thread and never in registerComponents(). I'm guessing it's Kotlin coroutine related and it occurs at seemingly random times. I'm going to try to restructure the code so that RequestManager (Glide.with()) is obtained before entering the coroutine, though I don't see why that matters. Entering a UI coroutine is not much different than posting to a handler, however, "not much" seems to be enough. Primarily I wanted to make you aware of this as I've not seen it reported. If I determine cause or workaround I'll update.

sjudd commented 6 years ago

Thanks for letting me know!

If there's a safer way we can initialize the singleton that works with coroutines I'm open to hearing about it. I'm surprised that a co-routine would allow a context switch in the middle of that method without an explicit call.

pandasys commented 6 years ago

Coroutine related is a guess. The stack trace I posted shows that particular exception does not occur within a coroutine. Here's an example of the issue arising within a coroutine in the UI context:

Fatal Exception: java.lang.IllegalStateException: You cannot call Glide.get() in registerComponents(), use the provided Glide instance instead
       at com.bumptech.glide.Glide.checkAndInitializeGlide(Glide.java:172)
       at com.bumptech.glide.Glide.get(Glide.java:160)
       at com.bumptech.glide.Glide.getRetriever(Glide.java:583)
       at com.bumptech.glide.Glide.with(Glide.java:632)
       at com.ealva.alvaplayer.ui.Drawers.expandAll(Drawers.kt:545)
       at com.ealva.alvaplayer.ui.Drawers.access$getDownloadMgr$p(Drawers.kt:99)
       at com.ealva.alvaplayer.ui.Drawers$updateAccountHeader$2.doResume(Drawers.kt:485)
       at kotlin.coroutines.experimental.jvm.internal.CoroutineImpl.resume(CoroutineImpl.kt:54)
       at kotlin.coroutines.experimental.jvm.internal.CoroutineImpl.resume(CoroutineImpl.kt:53)
       at kotlinx.coroutines.experimental.DispatchTask.run(CoroutineDispatcher.kt:123)
       at android.os.Handler.handleCallback(Handler.java:761)
       at android.os.Handler.dispatchMessage(Handler.java:98)
       at android.os.Looper.loop(Looper.java:156)
       at android.app.ActivityThread.main(ActivityThread.java:6523)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:942)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:832)
        try {
          Glide.with(this@AlvaPlayerApplicationImpl)
              .asDrawable()
              .apply(RequestOptions()
                         .placeholder(IconicsDrawable(this@AlvaPlayerApplicationImpl, MaterialDrawerFont.Icon.mdf_person)
                                          .color(skins.accentColor)
                                          .backgroundColor(skins.primaryColor)
                                          .sizeRes(R.dimen.material_drawer_account_header_compact)
                                          .paddingDp(16))
              )
              .load(uri)
              .into(imageView)
        } catch (e: IllegalStateException) {
          LOG.caught(LogLevel.ERROR, e)
        }
pandasys commented 6 years ago

tl;dr - Don't invoke any Kotlin code that can "suspend" during Glide initialization. I think any suspendable code in Glide user initialization, registerComponents(), creates a race condition.

This is an issue and I would expect it to arise as more people move to Kotlin. My solution of forcing init in Application.onCreate() is not fully tested but looks promising. I have to ensure Glide initialization before launching any coroutine.

Given both of our interpretation of Glide's code, how is this possible? It is coroutine related and becomes possible because of Glide's pluggable components and calling into client code during initialization. The synchronized lock on Glide.class is not sufficient in a Kotlin coroutine environment as it is a reentrant lock and coroutines are not threads. I'm not suggesting this is a Glide defect or that Glide needs to be modified, but we need to be aware of this. I need to research if I can wrap calls to Glide.with() to prevent suspension.

I'll try to be brief. When a coroutine comes to a suspension point, it can be replaced on a given thread with another coroutine and dispatched again sometime later. We're talking about the UI thread, so these particular coroutines will only dispatch on the UI thread. However, they can suspend. If a coroutine underlies Glide initialization somewhere, this scenario is possible. Also, the exception can be thrown from non-coroutine code in this scenario.

  1. Glide.with() is called in a UI coroutine and happened to be the first call to Glide - such as
    launch(UI) {
    Glide.with(activity)
    ...
    }

Glide begins its initialization and eventually calls out to user code to load factories. During factory loading Kotlin code is executed and hits a suspend point. At this time isInitializing = true; and we are in the initialzeGlide(context) method. The coroutine is suspended and a different coroutine or other UI code can execute. So, even when non-coroutine code executes, it hits the isInitializing flag and throws.

Coroutine suspend points are very common in coroutines executing on the UI thread as these coroutines often launch background work and await() the results. The await() method is a suspend point, which makes sense. Launch UI coroutine and

launch(UI) {
    val deferredSomething = async(CommonPool) {
      // io code
    }
    val something = deferredSomething.await()   // suspend point
    // make sure activity is good do UI work with something
    Glide.with(activity)  // Note: simplification here but representative of what occurs
        .load(something)
        ...
}

Detecting where this might occur may be non-trivial, as in my case. I use Kodein for dependency injection and my Glide modules depend on some injected items. At one time Kodein used coroutines as well. I use another library, MaterialDrawer, and it allows me to plug-in Glide for image loading and that may be invoked indirectly via a coroutine. That's just a couple interactions that bloom in several directions. The error occurs infrequently and seemingly at random times. If I get time I'll try to write a small app that duplicates the problem.

The workaround I'm trying required some changes to my dependency injection code (making some injection even lazier) and requires I put a Glide.get(context) in my Application.onCreate() method. Hopefully these suggestions save someone some time.

sjudd commented 6 years ago

Got it, yeah if a GlideModule implementation happens to call into a coroutine that suspends that makes perfect sense.

It seems like this isn't ideal behavior in coroutines, it's not particularly uncommon to have this kind of pattern where sections of code that should be executed atomically are run under a lock.

All that said, we could probably make this safer in Glide by using a Enum based singleton rather than using locks. It would require creating an inner wrapper that's just called by Glide, but that's not the end of the world. Note that I say safer, not safe, because it's possible there are other singletons or other usages of this pattern that would also need to be fixed.

pandasys commented 6 years ago

After my changes, I'm not getting reports from the field anymore.

For now, I'd say just throw a blurb in the documentation. If it becomes a frequent issue you could then be more defensive.

I think this is meant to address the issue: https://youtrack.jetbrains.com/issue/KT-17260 but would still be incumbent on the client (and is still a feature request). Maybe the way you could eventually address it is to provide Kotlin base classes where you call into client code and force a "nosuspend" policy during init.

A bit more info: https://github.com/Kotlin/kotlin-coroutines/issues/50

Thanks for the great library.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

mengshu commented 6 years ago

same problem: Android Studio 3.0.1 & Glide 4.5.0, java

01-19 09:38:41.826: W/System.err(2906): java.lang.IllegalStateException: You cannot call Glide.get() in registerComponents(), use the provided Glide instance instead 01-19 09:38:41.846: W/System.err(2906): at com.bumptech.glide.Glide.checkAndInitializeGlide(Glide.java:176) 01-19 09:38:41.846: W/System.err(2906): at com.bumptech.glide.Glide.get(Glide.java:164) 01-19 09:38:41.848: W/System.err(2906): at com.bumptech.glide.Glide.getRetriever(Glide.java:670) 01-19 09:38:41.849: W/System.err(2906): at com.bumptech.glide.Glide.with(Glide.java:697) 01-19 09:38:41.850: W/System.err(2906): at com.XXX.gamecenter.pad.imageload.ImageLoader.loadBanner(ImageLoader.java:275) 01-19 09:38:41.852: W/System.err(2906): at com.XXXgamecenter.pad.widget.MiImageView.bindData(MiImageView.java:35) 01-19 09:38:41.853: W/System.err(2906): at com.XXX.gamecenter.pad.widget.MainAdView$AdAdapter.getView(MainAdView.java:139)

sjudd commented 6 years ago

@mengshu does anything else mentioned in the issue apply to you? If not, please file a new issue. If you're using kotlin coroutines see above.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

jorgevila commented 6 years ago

Happening for me on Glide 4.6.1

 E  FATAL EXCEPTION: main
                         E  Process: ar.movistar.stvlauncher, PID: 11927
                         E  java.lang.IllegalStateException: You cannot call Glide.get() in registerComponents(), use the provided Glide instance instead
 E      at com.bumptech.glide.e.c(Glide.java:176)
 E      at com.bumptech.glide.e.a(Glide.java:164)
E      at com.bumptech.glide.e.e(Glide.java:670)
E      at com.bumptech.glide.e.b(Glide.java:697)

No couroutines involved... still under investigation.

Shouldn't be a way to initialize it before it is used without risk?

jorgevila commented 6 years ago

Finally, it was an error on applyOptions on a custom module, using Log.ASSERT which seems to be not accepted as a valid log level. It seems that failing inside such method, makes glide.with fail always.

sjudd commented 6 years ago

@jorgevila Could you provide a small code sample?

telenewbie commented 6 years ago

Caused by: java.lang.IllegalStateException: You cannot call Glide.get() in registerComponents(), use the provided Glide instance instead at com.bumptech.glide.Glide.checkAndInitializeGlide(Proguard:168) at com.bumptech.glide.Glide.get(Proguard:156) at com.bumptech.glide.Glide.getRetriever(Proguard:535) at com.bumptech.glide.Glide.with(Proguard:561) at com.txznet.loader.GlideApp.with(Proguard:73) at com.txznet.music.image.glide.GlideImageLoader.display(Proguard:123) at com.txznet.music.image.ImageFactory.display(Proguard:64) at com.txznet.music.widget.BarPlayerView.updatePlayInfo(Proguard:247) at com.txznet.music.ui.BaseBarActivity.onPlayInfoUpdated(Proguard:148) at com.txznet.music.ui.HomeActivity.onPlayInfoUpdated(Proguard:443) at com.txznet.music.ui.BaseBarPresenter.onPlayInfoUpdated(Proguard:151) at com.txznet.music.ui.BaseBarPresenter.register(Proguard:65) at com.txznet.music.ui.BaseBarActivity.onCreate(Proguard:61) at com.txznet.music.ui.HomeActivity.onCreate(Proguard:185) at android.app.Activity.performCreate(Activity.java:5494) at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1087) at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2148) ... 11 more

guangmomo commented 6 years ago

I not use kotlin, but I get this problem

012ABC commented 6 years ago

I not use kotlin, but I get this problem

minhhung2556 commented 6 years ago

@pandasys do you solve this issue? if yes, please show me how, I'm facing it now, :(( I tried every solution I can, but it does not work.

Tagakov commented 6 years ago

We just faced same problem. The cause was the exception thrown in registerComponents callback which have leading to inconsistent state where glide == null, isInitializing == true, and lock in synchronized (Glide.class) was released.

a-liYa commented 4 years ago

I have the same problem. When can this code be executed simultaneously, synchronized (Glide.class) { if (glide == null) { checkAndInitializeGlide(context); } }

nguyentrieuluat commented 4 years ago

I had the same proplem. Glide throws an exception when it is initialized on a background thread:

java.lang.IllegalArgumentException: You must call this method on the main thread at com.bumptech.glide.util.Util.assertMainThread(Util.java:142) at com.app.libimageloader.glide.AppGlideModule.registerComponents(BobaAppGlideModule.java:86) at com.bumptech.glide.GeneratedAppGlideModuleImpl.registerComponents(GeneratedAppGlideModuleImpl.java:35) at com.bumptech.glide.Glide.initializeGlide(Glide.java:273) at com.bumptech.glide.Glide.initializeGlide(Glide.java:223) at com.bumptech.glide.Glide.checkAndInitializeGlide(Glide.java:184) at com.bumptech.glide.Glide.get(Glide.java:168) at com.bumptech.glide.Glide.getRetriever(Glide.java:689) at com.bumptech.glide.Glide.with(Glide.java:716) at com.app.utils.NotificationHelper.extendWithBigPictureStyle(NotificationHelper.java:668)

nirajkulal commented 4 years ago

Any solution for this issue, anybody solved this?

rhasanr commented 4 years ago

I had the same proplem. Calling from recyclerview onBindViewHolder.

java.lang.IllegalStateException: You cannot call Glide.get() in registerComponents(), use the provided Glide instance instead

Any solution for this issue, anybody solved this?

vedang012 commented 3 years ago

Is there any other way to solve this issue? I am not using glide for android studio.

quibbler01 commented 6 months ago

When you use GlideAppModule, from docs, you should use: GlideApp.with(mContext)

Instead of: Glide.with(mContext)