-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
NetworkOnMainThreadException when using Glide with OkHttp #257
Comments
See square/okhttp#869 for more context. |
@sjudd So what would be the best temp fix in this case in your opinion? |
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 |
By ignoring you mean changing ThreadPolicy or try-catch block or maybe something else? |
@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 |
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
|
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. |
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. |
Ok, thanks :) |
@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. |
That's why he opened the other bug, to solve it in the library (when calling |
FYI, OkHttp now cancels without the warning. |
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. |
It's best in OkHttp 2.7 and 3.0. |
@swankjesse Is there a way to detect |
Not specifically. Let’s just do canceling for 3.x and above. |
Progress toward bumptech#257
Fixed in master here: 91c75ee |
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:
Exception trace:
The text was updated successfully, but these errors were encountered: