Skip to content

Commit

Permalink
fix(image-loader): queue stucks processing same url more than concurr…
Browse files Browse the repository at this point in the history
…ency

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.
  • Loading branch information
Jonas Schumacher committed Jul 15, 2018
1 parent 3ce31c0 commit 12e7750
Showing 1 changed file with 36 additions and 19 deletions.
55 changes: 36 additions & 19 deletions src/providers/image-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
}
);
});
Expand All @@ -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
Expand Down

0 comments on commit 12e7750

Please sign in to comment.