From 12e7750df64ac06916181aedfbf978d7d4ba8eb9 Mon Sep 17 00:00:00 2001 From: Jonas Schumacher Date: Wed, 11 Jul 2018 15:23:01 -0300 Subject: [PATCH] fix(image-loader): queue stucks processing same url more than concurrency When loading the same url more than the allowed by 'concurrency', the queue may gets stuck, because the logic to prevent same image from loading at same time doesn't release the 'processing' counter , which is incremented always, even when processing a url that is already being processed. --- src/providers/image-loader.ts | 55 +++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/src/providers/image-loader.ts b/src/providers/image-loader.ts index d619691..1980105 100644 --- a/src/providers/image-loader.ts +++ b/src/providers/image-loader.ts @@ -248,29 +248,31 @@ export class ImageLoader { // take the first item from queue const currentItem: QueueItem = this.queue.splice(0, 1)[0]; + + // function to call when done processing this item + // this will reduce the processing number + // then will execute this function again to process any remaining items + const done = () => { + this.processing--; + this.processQueue(); + + // only delete if it's the last/unique occurrence in the queue + if (this.currentlyProcessing[currentItem.imageUrl] !== undefined && !this.currentlyInQueue(currentItem.imageUrl)) { + delete this.currentlyProcessing[currentItem.imageUrl]; + } + }; + + const error = (e) => { + currentItem.reject(); + this.throwError(e); + done(); + }; + if (this.currentlyProcessing[currentItem.imageUrl] === undefined) { this.currentlyProcessing[currentItem.imageUrl] = new Promise((resolve, reject) => { // process more items concurrently if we can if (this.canProcess) this.processQueue(); - // function to call when done processing this item - // this will reduce the processing number - // then will execute this function again to process any remaining items - const done = () => { - this.processing--; - this.processQueue(); - - if (this.currentlyProcessing[currentItem.imageUrl] !== undefined) { - delete this.currentlyProcessing[currentItem.imageUrl]; - } - }; - - const error = (e) => { - currentItem.reject(); - this.throwError(e); - done(); - }; - const localDir = this.file.cacheDirectory + this.config.cacheDirectoryName + '/'; const fileName = this.createFileName(currentItem.imageUrl); @@ -294,11 +296,13 @@ export class ImageLoader { }).catch((e) => { //Could not write image error(e); + reject(e); }); }, (e) => { //Could not get image via httpClient error(e); + reject(e); } ); }); @@ -307,11 +311,24 @@ export class ImageLoader { this.currentlyProcessing[currentItem.imageUrl].then(() => { this.getCachedImagePath(currentItem.imageUrl).then((localUrl) => { currentItem.resolve(localUrl); - }) + }); + done(); + }, + (e) => { + error(e); }); } } + /** + * Search if the url is currently in the queue + * @param imageUrl {string} Image url to search + * @returns {boolean} + */ + private currentlyInQueue(imageUrl: string) { + return this.queue.some(item => item.imageUrl === imageUrl); + } + /** * Initialize the cache service * @param replace {boolean} Whether to replace the cache directory if it already exists