mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2025-04-21 07:56:16 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

utils: Fix promise creation accounting bug in promises.timesLimit

Browse source 
Before this change, `promises.timesLimit()` created `concurrency - 1`
too many promises. The only users of this function use a concurrency
of 500, so this meant that 499 extra promises were created each time
it was used. The bug didn't affect correctness, but it did result in a
large number of unnecessary database operations whenever a pad was
deleted. This change fixes that bug.

Also:
  * Convert the function to async and have it resolve after all of the
    created promises are resolved.
  * Reject concurrency of 0 (unless total is 0).
  * Document the function.
  * Add tests.
This commit is contained in:
Richard Hansen 2020-09-16 16:59:00 -04:00 • committed by John McLear
parent 65942691b6
commit 346111250e
2 changed files with 101 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

38
src/node/utils/promises.js
View file

@ -35,27 +35,21 @@ exports.firstSatisfies = (promises, predicate) => {
return Promise.race(newPromises);
};
exports.timesLimit = function(ltMax, concurrency, promiseCreator) {
var done = 0
var current = 0
function addAnother () {
function _internalRun () {
done++
if (done < ltMax) {
addAnother()
}
}
promiseCreator(current)
.then(_internalRun)
.catch(_internalRun)
current++
}
for (var i = 0; i < concurrency && i < ltMax; i++) {
addAnother()
// Calls `promiseCreator(i)` a total number of `total` times, where `i` is 0 through `total - 1` (in
// order). The `concurrency` argument specifies the maximum number of Promises returned by
// `promiseCreator` that are allowed to be active (unresolved) simultaneously. (In other words: If
// `total` is greater than `concurrency`, then `concurrency` Promises will be created right away,
// and each remaining Promise will be created once one of the earlier Promises resolves.) This async
// function resolves once all `total` Promises have resolved.
exports.timesLimit = async (total, concurrency, promiseCreator) => {
if (total > 0 && concurrency <= 0) throw new RangeError('concurrency must be positive');
let next = 0;
const addAnother = () => promiseCreator(next++).finally(() => {
if (next < total) return addAnother();
});
const promises = [];
for (var i = 0; i < concurrency && i < total; i++) {
promises.push(addAnother());
}
await Promise.all(promises);
}