Complex synchronization issue. #402
Labels
Comments
|
Do you exclusively load images using into(width, height) on background threads? Can you try running Thread.getAllStackTraces() when you detect (or suspect) deadlock and provide those stack traces? It also looks like sending SIGQUIT will dump stack traces to /data/anr/traces.txt. See: https://groups.google.com/forum/#!topic/android-developers/N18KV5DyJpE |
|
The only into(w,h) is called from the onLoad of the DataFetcher The ImageRequest modelloader is only : @Override
public DataFetcher<InputStream> getResourceFetcher(ImageRequest model, int width, int height) {
return new ImageRequestCacheStreamFetcher(mContext, mCacheDirectory, model);
}The DataFetcher @Override
public InputStream loadData(Priority priority) throws Exception {
test if image is from cache then return inputstream from it (never locks)
The glide.into that will lock forever even with timeouts in the okhttpclient unless I force the timeouts in the .get()
put image in cache (never locks here)
return inputstream from image in cache.
}The first Glide call is from a cursorAdapter with a simple : Glide.with(ctx)
.load(DisplayHelper.getGlideImageRequest(buffer))
.centerCrop()
.placeholder(mDefaultImageResource)
.error(mDefaultErrorResource)
.crossFade(250)
.into((ImageView) view.getTag(imageViewId));I suspect this is more or less tied to my PR about request not set on the future soon enough. No more time today to roll back my current state with my implementation for the stack trace, will try to gather them tomorrow. |
|
I agree, your PR is probably related. It looks like at least the logic here is broken: https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java#L673. The run() method there should first acquire a lock on the Target so that it isn't cancelled between the isCanceled check and the into() method. |
|
|
|
Here's the part relative to glide on the traces. 2 different cases but always blocked queues so clearly a synchronization problem. |
|
@sjudd this is not fully fixed :( There's no more infinite locks so situation is a little better but seems you only fixed one of the reported traces. Now there's sometimes (often) lock of the duration of .get(10, TimeUnit.SECONDS); where all images will not be downloaded and will all return null after, then some other images do download then hang again.. |
|
@Tolriq Are the downloads timing out? Or are they succeeding but we're still blocking on the future for no good reason? Do you have repro steps? Thanks for confirming we've at least fixed the deadlock. |
|
It's seems to be still FutureRequests since I do set a 5 second timeout on okhttp and it timeout at 10. Seems a reuse of something does trigger the problem. The repro steps are as mentioned earlier. Register a modelloader for something then in it's dataFetcher onLoad() call mNetworkLoading = Glide.with(ctx)
.load(new NetworkImageRequest(imageRequest))
.asBitmap()
.diskCacheStrategy(DiskCacheStrategy.NONE)
.into(500, 500);
image = mNetworkLoading.get(10, TimeUnit.SECONDS);The NetworkImageRequest model handler is a copy paste of the okhttp integration library with just some specific headers handling. Then in a normal adapter call a simple Glide.load(newmodel).into(imgview) and scroll. ImageRequestCacheStreamFetcher.loadData@115: Error : Cancelled :
java.lang.InterruptedException
at java.lang.Object.wait(Native Method)
at java.lang.Object.wait(Object.java:422)
at com.bumptech.glide.request.RequestFutureTarget$Waiter.waitForTimeout(RequestFutureTarget.java:238)
at com.bumptech.glide.request.RequestFutureTarget.doGet(RequestFutureTarget.java:183)
at com.bumptech.glide.request.RequestFutureTarget.get(RequestFutureTarget.java:108)
at org.leetzone.android.yatsewidget.glide.ImageRequestCacheStreamFetcher.loadData(ImageRequestCacheStreamFetcher.java:112)
at org.leetzone.android.yatsewidget.glide.ImageRequestCacheStreamFetcher.loadData(ImageRequestCacheStreamFetcher.java:33)
at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.loadData(ImageVideoModelLoader.java:70)
at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.loadData(ImageVideoModelLoader.java:53)
at com.bumptech.glide.load.engine.DecodeJob.decodeSource(DecodeJob.java:170)
at com.bumptech.glide.load.engine.DecodeJob.decodeFromSource(DecodeJob.java:128)
at com.bumptech.glide.load.engine.EngineRunnable.decodeFromSource(EngineRunnable.java:122)
at com.bumptech.glide.load.engine.EngineRunnable.decode(EngineRunnable.java:101)
at com.bumptech.glide.load.engine.EngineRunnable.run(EngineRunnable.java:58)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:422)
at java.util.concurrent.FutureTask.run(FutureTask.java:237)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
at java.lang.Thread.run(Thread.java:818)
at com.bumptech.glide.load.engine.executor.FifoPriorityThreadPoolExecutor$DefaultThreadFactory$1.run(FifoPriorityThreadPoolExecutor.java:52) |
|
Ok two things here:
To expand on #2 a bit: Glide uses a variable sized thread pool based on the number of cores a device has. In the worst case, the pool will use a single thread. By nesting Glide calls as you are, it's possible you will ask that single pool thread to block and allow no other work, until that single pool thread does some other work, which is deadlock. For pools with sizes > 1, deadlock is less likely, but it can still happen. You'll need to try to find a way to avoid calling into Glide in ImageRequestCacheStreamFetcher. If you provide a source snippet I may be able to suggest an alternative. |
|
Well there's 4 thread in the pool as the stack traces shows and if I understand correctly and it happens each time. My need have not changed since the beginning :( I need to have a permanent SOURCE cache with images that are stored not at SOURCE size but at a max size depending on device (#383) From all previous discussions the easiest way was to do it in the loadData from the fetcher. Until there's a way to have a way to intercept cache writes / read with more details about what is cached (needs model + cache strategy) I do not think there's other ways. If it's now clear that calling glide inside glide is not supported I'll keep my way as it works :) To me there may just be some missing parts in the doc about those details :( |
|
FWIW we added One better would be to make this happen automatically by checking the thread that Glide's started on, but that's a bit more complicated and there doesn't seem to be a lot of demand for it at the moment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Glide Version/Integration library (if any): 3.6.0-SNAPSHOT
Device/Android Version: Nexus 5 / 5.1.0
Issue details/Repro steps:
In an attempt to handle my dual cache problem while using all the power of Glide I encounter a synchronization problem inside Glide but quite out of my competence on this library to find and understand.
The principle is quite simple :
Register 2 modules : 1 for normal queries that will then call Glide synchronously with the other model loader in it's DataFetcher.onLoad
Glide load line in the onLoad:
The NetworkImageRequest is a module that is a copy paste of the okhttp integration.
When using glide like this somtimes when scrolling fast or not, (I.E. randomly) there will be locks that will prevent all future image loading to happens and images to not be displayed.
Adding a forced timeout (
.get(20, TimeUnit.SECONDS);) will make things recover after the timeout but for multiple images they will not be displayed and sometimes cache is corrupted.And no image loading during xx seconds is not really wanted for smooth interface.
Remark : There's a timeout set on OKHTTP client that do not trigger so the lock is not on OKHTTP .
If instead of calling Glide I use my code to download the image with okhttp and resize it at the exact same place all works perfectly and there's never locks.
So all points to a non reentrant lock somewhere in Glide but I have no clue where to look at :(
The text was updated successfully, but these errors were encountered: