search

bumptech / glide

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

NetworkOnMainThreadException when using Glide with OkHttp #257

Closed thorbenprimke closed 8 years ago

thorbenprimke commented 10 years ago

After upgrading to 3.4, it intermittently hits a NetworkOnMainThreadException. I haven't had time to investigate it deeper except for removing the OkHttp registration line.

Init code: Glide.get(this).register(GlideUrl.class, InputStream.class, new OkHttpUrlLoader.Factory());

Calling code: 

Glide.with(activity)
    .load(uri)
    .centerCrop()
    .into(imageView);

Exception trace: 

android.os.NetworkOnMainThreadException
        at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1145)
        at com.android.org.conscrypt.OpenSSLSocketImpl.close(OpenSSLSocketImpl.java:1009)
        at com.squareup.okhttp.Connection.closeIfOwnedBy(Connection.java:136)
        at com.squareup.okhttp.OkHttpClient$1.closeIfOwnedBy(OkHttpClient.java:70)
        at com.squareup.okhttp.internal.http.HttpConnection.closeIfOwnedBy(HttpConnection.java:134)
        at com.squareup.okhttp.internal.http.HttpTransport.disconnect(HttpTransport.java:154)
        at com.squareup.okhttp.internal.http.HttpEngine.disconnect(HttpEngine.java:447)
        at com.squareup.okhttp.Call.cancel(Call.java:126)
        at com.squareup.okhttp.Dispatcher.cancel(Dispatcher.java:134)
        at com.squareup.okhttp.OkHttpClient.cancel(OkHttpClient.java:506)
        at com.bumptech.glide.integration.okhttp.OkHttpStreamFetcher.cancel(OkHttpStreamFetcher.java:59)
        at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.cancel(ImageVideoModelLoader.java:120)
        at com.bumptech.glide.load.engine.DecodeJob.cancel(DecodeJob.java:132)
        at com.bumptech.glide.load.engine.EngineRunnable.cancel(EngineRunnable.java:46)
        at com.bumptech.glide.load.engine.EngineJob.cancel(EngineJob.java:119)
        at com.bumptech.glide.load.engine.EngineJob.removeCallback(EngineJob.java:93)
        at com.bumptech.glide.load.engine.Engine$LoadStatus.cancel(Engine.java:52)
        at com.bumptech.glide.request.GenericRequest.cancel(GenericRequest.java:283)
        at com.bumptech.glide.request.GenericRequest.clear(GenericRequest.java:301)
        at com.bumptech.glide.GenericRequestBuilder.into(GenericRequestBuilder.java:606)
        at com.bumptech.glide.GenericRequestBuilder.into(GenericRequestBuilder.java:651)
        at com.bumptech.glide.DrawableRequestBuilder.into(DrawableRequestBuilder.java:436)
        at (ImageLoader.java:182)
        at $ListAdapter.getView(*ListController.java:286)
        at android.widget.HeaderViewListAdapter.getView(HeaderViewListAdapter.java:220)
        at android.widget.AbsListView.obtainView(AbsListView.java:2255)
        at android.widget.ListView.makeAndAddView(ListView.java:1790)
        at android.widget.ListView.fillDown(ListView.java:691)
        at android.widget.ListView.fillSpecific(ListView.java:1349)
        at android.widget.ListView.layoutChildren(ListView.java:1608)
        at android.widget.AbsListView.onLayout(AbsListView.java:2087)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at com.android.internal.widget.ActionBarOverlayLayout.onLayout(ActionBarOverlayLayout.java:374)
        at android.view.View.layout(View.java:14817)
        at android.view.ViewGroup.layout(ViewGroup.java:4631)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)

sjudd commented 10 years ago

See https://github.com/square/okhttp/issues/869 for more context.

RobertoArtiles commented 9 years ago

@sjudd So what would be the best temp fix in this case in your opinion?

sjudd commented 9 years ago

It's pretty clear from the icebox designation that their not planning on fixing the bug in OkHttp.

I'm inclined to take @swankjesse at his word and assume that this call is non-blocking and won't have any negative impact on the performance of the library, in which case we can also just ignore this warning.

If you think this call is impacting your application, I'd be curious to see timings indicating how long we're spending in cancel() (traceview might help here). We could consider posting cancellations to a background thread in OkHttpStreamFetcher, but doing so may introduce other performance problems.

RobertoArtiles commented 9 years ago

By ignoring you mean changing ThreadPolicy or try-catch block or maybe something else?

sjudd commented 9 years ago

@RobertoArtiles We don't set a strict mode thread policy, if you do so in your app you may want to consider changing it so that it does not throw for this kind of exception. Try/catch sounds ugly and would probably be hard to write effectively.

Do also feel free to try posting cancel() to a background thread in OkHttpStreamFetcher to avoid the problem. If you don't notice a performance impact and you are concerned about the strict mode error then forking the OkHttp integration library is not an unreasonable course of action.

RobertoArtiles commented 9 years ago

I don't set ThreadPolicy myself. NetworkOnMainThreadException is the exception which system throws (if app targets API 11+) when you do networking operations on the the main thread. The question is not about the performance, but about this exception which leads to the app crash and which you guaranteed to get during a ListView fast scroll (I don't call cancel() my self). And I'm trying to think of the best solution in this case.

android.os.NetworkOnMainThreadException
    at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1145)
    at com.android.org.conscrypt.OpenSSLSocketImpl.close(OpenSSLSocketImpl.java:1009)
    at com.squareup.okhttp.Connection.void closeIfOwnedBy(java.lang.Object)(:136)
    at com.squareup.okhttp.OkHttpClient$1.void closeIfOwnedBy(com.squareup.okhttp.Connection,java.lang.Object)(:70)
    at com.squareup.okhttp.internal.http.HttpConnection.void closeIfOwnedBy(java.lang.Object)(:134)
    at com.squareup.okhttp.internal.http.HttpTransport.void disconnect(com.squareup.okhttp.internal.http.HttpEngine)(:132)
    at com.squareup.okhttp.internal.http.HttpEngine.void disconnect()(:465)
    at com.squareup.okhttp.Call.void cancel()(:124)
    at com.squareup.okhttp.Dispatcher.void cancel(java.lang.Object)(:134)
    at com.squareup.okhttp.OkHttpClient.com.squareup.okhttp.OkHttpClient cancel(java.lang.Object)(:506)
    at com.bumptech.glide.integration.okhttp.OkHttpStreamFetcher.void cancel()(:59)
    at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.void cancel()(:120)
    at com.bumptech.glide.load.engine.DecodeJob.void cancel()(:132)
    at com.bumptech.glide.load.engine.EngineRunnable.void cancel()(:46)
    at com.bumptech.glide.load.engine.EngineJob.void cancel()(:119)
    at com.bumptech.glide.load.engine.EngineJob.void removeCallback(com.bumptech.glide.request.ResourceCallback)(:93)
    at com.bumptech.glide.load.engine.Engine$LoadStatus.void cancel()(:52)
    at com.bumptech.glide.request.GenericRequest.void cancel()(:283)
    at com.bumptech.glide.request.GenericRequest.void clear()(:301)
    at com.bumptech.glide.GenericRequestBuilder.com.bumptech.glide.request.target.Target into(com.bumptech.glide.request.target.Target)(:606)
    at com.bumptech.glide.GenericRequestBuilder.com.bumptech.glide.request.target.Target into(android.widget.ImageView)(:651)
    at com.bumptech.glide.DrawableRequestBuilder.com.bumptech.glide.request.target.Target into(android.widget.ImageView)(:436)
    at com.package_name.FriendView.void setBaseUserData(com.package_name.model.User)(:38)
    at com.package_name.FriendsOverlayView$FriendsAdapter.void bindView(android.view.View,android.content.Context,android.database.Cursor)(:456)
    at android.widget.CursorAdapter.getView(CursorAdapter.java:254)
    at com.commonsware.cwac.merge.MergeAdapter.android.view.View getView(int,android.view.View,android.view.ViewGroup)(:272)
    at android.widget.HeaderViewListAdapter.getView(HeaderViewListAdapter.java:220)
    at android.widget.AbsListView.obtainView(AbsListView.java:2255)
    at android.widget.ListView.makeAndAddView(ListView.java:1790)
    at android.widget.ListView.fillDown(ListView.java:691)
    at android.widget.ListView.fillGap(ListView.java:655)
    at android.widget.AbsListView.trackMotionScroll(AbsListView.java:5143)
    at android.widget.AbsListView$FlingRunnable.run(AbsListView.java:4254)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:761)
    at android.view.Choreographer.doCallbacks(Choreographer.java:574)
    at android.view.Choreographer.doFrame(Choreographer.java:543)
    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:747)
    at android.os.Handler.handleCallback(Handler.java:733)
    at android.os.Handler.dispatchMessage(Handler.java:95)
    at android.os.Looper.loop(Looper.java:136)
    at android.app.ActivityThread.main(ActivityThread.java:5086)
    at java.lang.reflect.Method.invokeNative(Native Method)
    at java.lang.reflect.Method.invoke(Method.java:515)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit
sjudd commented 9 years ago

Ah right, in that case it's set by default in dev builds. You can override it yourself manually.

Again the "best" solution here would be to post cancel to a background thread to work around the okhttp issue. Doing so should be relatively straightforward if you're interested. I'd be happy to see a pull request.

sjudd commented 9 years ago

Actually I just looked back at our implementation for HttpUrlConnections, and we don't call cancel to avoid this exception: https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java#L107.

I'd been assuming this was limited to OkHttp, but since it's not it's probably worth considering more broadly.

For now it seems like we can probably just not call cancel at all.

RobertoArtiles commented 9 years ago

Ok, thanks :)

swankjesse commented 9 years ago

@sjudd I think posting the cancel for a background thread is the best solution, but it should be the application's job to do that. Not OkHttp's.

TWiStErRob commented 9 years ago

That's why he opened the other bug, to solve it in the library (when calling DataFetcher.cancel) regardless of what I/O integration method you use.

swankjesse commented 8 years ago

FYI, OkHttp now cancels without the warning.

TWiStErRob commented 8 years ago

Both okhttp3 integration libs (v3 and v4) can call cancel safely now, according to @swankjesse. Reopened so the commented out code can be reverted at least for those new versions.

@swankjesse do you know since when it is safe to call it and if we can detect if the version on the classpath is newer than that (preferably in a proguard-safe way)? This would be useful for the okhttp2 integration libs.

swankjesse commented 8 years ago

It's best in OkHttp 2.7 and 3.0.

TWiStErRob commented 8 years ago

@swankjesse Is there a way to detect [2.7, 3.0)?

swankjesse commented 8 years ago

Not specifically. Let’s just do canceling for 3.x and above.

sjudd commented 8 years ago

Fixed in master here: https://github.com/bumptech/glide/commit/91c75ee589d0c57aa114734a3d5ff7d0571be6fe