rogierschouten / async-lock Goto Github PK
View Code? Open in Web Editor NEWThis project forked from rain1017/async-lock
Lock on asynchronous code for Nodejs
License: MIT License
This project forked from rain1017/async-lock
Lock on asynchronous code for Nodejs
License: MIT License
I want to limit my lock, rather than running 1 at a time, i wanted it to run 10 at a time. Is this possible?
Examine the following code:
const AsyncLock = require("async-lock");
const lock = new AsyncLock({ timeout: 5000 });
lock.acquire("test", function(done) {}); // hold the lock open
const promise = lock.acquire("test", function() {}); // try to acquire the busy lock
promise.then(() => console.log("success"));
promise.catch(() => console.log("fail"));
What do you expect the program to output? I was expecting "fail", but the reality was:
$ node test.js
fail
(node:20581) UnhandledPromiseRejectionWarning: Error: async-lock timed out
at Timeout._onTimeout (/home/vwoo/shared/autocomplete/node_modules/async-lock/lib/index.js:164:17)
at ontimeout (timers.js:436:11)
at tryOnTimeout (timers.js:300:5)
at listOnTimeout (timers.js:263:5)
at Timer.processTimers (timers.js:223:10)
(node:20581) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:20581) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
It is very strange to me that we have both the "fail" message as well as an unhandled promise rejection error. I'm not sure why and I'll try to take a peek inside the library to understand (just documenting my findings here).
Seems like the Promise
global is pretty standard now, it would be nice if that were used as the default instead of Q, and so that this package has no dependencies.
Does one failed task fail all queued tasks?
I modified the first test by adding a long task and it was seen that other tasks also failed.
it('Single key test', function (done) {
var lock = new AsyncLock({ timeout: 10 });
var taskCount = 8;
var keyCount = 4;
var finishedCount = 0;
var isRunning = {};
var taskNumbers = [];
for (var i = 0; i < taskCount; i++) {
taskNumbers.push(i);
}
var key = 1 % keyCount;
lock.acquire(key, function (cb) {
console.log('long(key%d) start', key);
setTimeout(cb, 12);
}).then(
() => console.log('long(key%d) done', key),
err => console.log('long(key%d) error', key, err)
);
taskNumbers.forEach(function (number) {
var key = number % keyCount;
lock.acquire(key, function (cb) {
assert(!isRunning[key]);
assert(lock.isBusy() && lock.isBusy(key));
var timespan = Math.random() * 1000;
console.log('task%s(key%s) start, %s ms', number, key, timespan);
setTimeout(cb.bind(null, null, number), timespan);
}, function (err, result) {
if (err) {
// return done(err);
}
console.log('task%s(key%s) done %s', number, key, !err ? '' : err.message || err);
isRunning[key] = false;
finishedCount++;
if (finishedCount === taskCount) {
lock.acquire(['key1','key2'], () => { done(); });
// assert(!lock.isBusy());
// done();
}
});
});
});
And run:
$ ./node_modules/.bin/mocha -f "Single key test"
AsyncLock Tests
long(key1) start
task0(key0) start, 926.8097332784681 ms
task2(key2) start, 232.97781253882687 ms
task3(key3) start, 631.8339886981712 ms
task1(key1) done async-lock timed out
task4(key0) done async-lock timed out
task5(key1) done async-lock timed out
task6(key2) done async-lock timed out
task7(key3) done async-lock timed out
long(key1) done
task2(key2) done
task3(key3) done
task0(key0) done
โ Single key test (931ms)
1 passing (938ms)
Only tasks started before the long task timed out has been complete without error.
When I started using this library I assumed that the order of keys wouldn't matter when acquiring multiple locks but I was wrong as demonstrated by the example below:
let AsyncLock = require('async-lock');
let lock = new AsyncLock();
lock.acquire(["a", "b"], (release)=>{
console.log("1");
setTimeout(release);
});
lock.acquire(["b", "c"], (release)=>{
console.log("2");
setTimeout(release);
});
lock.acquire(["c", "d"], (release)=>{
console.log("3");
setTimeout(release);
});
I was expecting it to print 1 -> 2 -> 3 but it prints 1 -> 3 -> 2 because lock.acquire(["b", "c"],
only queues "c" after "b" has been acquired. I think that this behavior is counter intuitive and should be explained in the documentation.
Running this on Node v10 gives:
(node:28588) Warning: Promise.defer is deprecated and will be removed in a future version. Use new Promise instead.
I see that some Promise usage was cleared up in #2, but I think the code needs to remove all use of Promise.defer()
in favour of new Promise(...)
.
I have two suggestions for the library:
var lock = new AsyncLock({maxPending: 1});
lock.acquire(key, fn1, cb); // runs immediately
lock.acquire(key, fn2, cb); // added to queue (reached maxPending of 1)
lock.acquire(key, priorityFn, cb, {skipQueue: true}); // jumps queue and basically replaces fn2
Hi @abozaralizadeh and @ThePiz apparently it slipped our attention but the unit test for maxOccupationtTime fails rightfully. Any chance for you to check this out?
Error:(8, 40) TS2345: Argument of type '{ maxOccupationTime: number; }' is not assignable to parameter of type 'AsyncLockOptions'. Object literal may only specify known properties, and 'maxOccupationTime' does not exist in type 'AsyncLockOptions'.
@rogierschouten
This works perfectly locally but when it doesn't work on the live, which has 2 kubernetes pods handled by load balancer. It will be no problem with single pod but you know, usually in live environment we have multiple processes running to handle concurrent requests in a distributed way, like in my example or on heroku with multiple dynos.
Is there any solution to overcome this problem?
Thanks in advance and stay safe.
I am working on a detecting if there are pending tasks on a queue, so I checked lock.queues
. However we don't make this public knoweldge so it's implementation detail right now. Is it ok to pr to make this public and kind of a contract that we stick to?
Hi,
I just wanted to make sure the lock will work as expected if one calls it this way:
import AsyncLock from 'async-lock';
const lock = new AsyncLock();
async function bla () {
await lock.acquire(bla.name, async () => {
// some async stuff that should be executed in critical section
});
}
In my music scheduler application, mutex is required to ensure the music is played as scheduled without conflict which may be caused by scheduler plan update from server. So after each song is over, I lock the generation function of the next music. However, after a few hours (3 ~ 4hours, nearly 100 songs, more than 2000 mutex operation), a deadlock always occurred in production environment.
So I switch to use other mutex library, and no deadlock occurred since then.
The following is the structure of my code.
let globalMusicPlayerAudio = new Audio();
let musicSchedulerLock = new AsyncLock();
globalMusicPlayerAudio.onloadedmetadata = (e) => {
// show music title only when music is load
eleMusicName.innerHTML = globalMusicMetaNow.title + ' - ' + globalMusicMetaNow.artist
// some other log
};
globalMusicPlayerAudio.ontimeupdate = (e) => {
// calculate current progress and update progress bar
}
globalMusicPlayerAudio.onended = (e) => {
// state update.
timerNextMusic();
}
globalMusicPlayerAudio.onerror = (e) => {
deleteLocalFile();
timerNextMusic();
}
function timerNexMusic() {
let taskAfterUnlock = () => {};
musicSchedulerLock.acquire(['globalMusicUpdateLock'], function (done) {
try {
// log state of current music .
if (nowIsPlayingAlready) {
return
}
// detect available music . for example, check whether the file exists, whether it is currently playing time, whether the playlist should be switched, etc.
var findNextMusic = undefined; // this is the result
if (findNextMusic == null) {
// no need to play
} else {
taskAfterUnlock = () {
globalMusicPlayerAudio.src = findNextMusic.src;
globalMusicPlayerAudio.play();
}
}
} catch(e) {
// error logger
} finally {
done();
}
}, (err, ret) => {
if (taskAfterUnlock != null) {
console.log('Music Timer. Do something after lock release')
taskAfterUnlock()
}
if (globalMusicTimer) {
clearTimeout(globalMusicTimer)
}
globalMusicTimer = setTimeout(timerNexMusic, 60 * 1000);
})
}
We use this package in a browser, and we have a problem with the build: the package uses arrow functions that are not ES compliant.
Temporarily we solved the issue by adding the path "node_modules/async-lock/**/*.js"
to our tsconfig path (plus allowJs: true
) but it's rather a hack than a real solution.
I see the arrow function in 2 lines which could be changed to a simple function without a problem. (I did not check for anything else that is not ES5 compliant.)
We have no problem with Promises and other things that can be polyfilled.
is it possible to check if the key is already locked and instead of waiting it until it's released, execute another function ?
Hi,
Am I correct that the expected behavior of the maxOccupationTime option is that once the lock has been acquired, the maxOccupationTime timer starts counting for the item that has acquired the lock? What I think I am observing in practice that this maxOccupationTime timer beginning as soon as lock.acquire is called and it enters the queue. This behavior is what I expect from the timeout option. Is my understanding of a lock being acquired incorrect?
My code behaves as follows:
const lock = new AsyncLock({
maxOccupationTime: 10 * 1000 // 10 seconds
});
for (let index = 0; index < 100; index += 1) {
lock.acquire("abc", async () => {
// code that takes about 1 second to execute and returns at the end
})
}
I may be doing something obviously wrong, I'm not sure. But I tried putting a lock around some code in a promise that was causing our server issues to be sure it was only done once at a time. However from my logs I see multiple clients ending up in the locked section simultaneously.
This is the function in question: https://github.com/gcp/leela-zero-server/blob/287ae0a852ff03a2cf0d528ee2c7c6981ec33c36/server.js#L182
The bulk of the Promise has the code wrapped in the lock.request(). But I can see extra clients are able to acquire the lock and start an unzip process even before the first unzip has completed.
Does the lock have to be outside the "return new Promise" above it? Do I need to "await" the lock.request() when called inside a Promise?
Any help appreciated. Thanks!
Is there a reason why the acquireBatch function reverses the keys here? The problem is Array.prototype.reverse is a destructive function that (unexpectedly) modifies the original array. I had a downstream bug that I just tracked back to this behavior.
At the very least, could the line above be changed to something like [...keys].reverse()
or something? And yes, I know I can work around this behavior by doing the same up in my code but this behavior doesn't seem desirable.
I will be happy if you support typescript ๐
My Heroku App is running on multiple dynos. Is this package able to lock across dynos?
Is there a reason for it? Why is it not Infinity?
Hi @rogierschouten! Thanks for keeping to work on this library, really appreciate ๐
Correct me if I'm wrong, currently the only way to distinguish between async-lock errors and any error that's thrown by the wrapped async function is by using the error message:
function someAsyncFn() {}
lock.acquire(key, someAsyncFn, opts)
.catch(function(err) {
const isAsyncLockTimeoutError = err instanceof Error && err.message.includes('async-lock timed out')
if (isAsyncLockTimeoutError) {
// handle async-lock timeout error
}
});
IMHO it would be great if async-lock could deal with its own error classes to model anything can go wrong on the library side.
class AsyncLockError extends Error {
constructor(message) {
super(message);
}
}
class AsyncLockTimeoutError extends AsyncLockError {
constructor(queueKey) {
super(`Async lock timeout out in queue ${async-lock timed out in queue}`);
this.name = 'AsyncLockTimeoutError';
}
}
This mean on the user-land side it would be much cleaner handling any async-lock error:
function someAsyncFn() {}
lock.acquire(key, someAsyncFn, opts)
// Handle different kind of async-lock errors
.catch(function(err) {
if (err instanceof AsyncLockTimeoutError) {
// do something...
}
if (err instanceof AsyncLockMaxOccTimeError) {
// do something...
}
});
function someAsyncFn() {}
lock.acquire(key, someAsyncFn, opts)
// Catch any kind of async-lock errors
.catch(function(err) {
if (err instanceof AsyncLockError) {
// async-lock error
} else {
// Any error thrown by `someAsyncFn`
}
});
What do you think about that? I can find some time to work on this if it can help.
OK, what I did was bullshit, but I tried to hang the fn for a certain time and see, if the lock works, if acquired from another side.
lock.acquire("postman", (id) => {
var e = new Date().getTime() + (10 * 1000);
while (new Date().getTime() <= e) { }
}, (err, ret) => {
if (err) console.log(err);
))
What I see now is, that after the first hang for 10 secs all subsequent requests to lock.acquire end up in err Error: async-lock timed out
I know this case is pathologic, but why is the lock running into the timeout after?
or any plan to add this feature?
Thank you.
Jerry
Please consider this module for use as an example on the project site.
I hate that it requires another module but with a little modification by you that will be gone.
It was so hard to figure out how to put this module into the async() await form that maybe this could be a guide for the next adopter.
-Mike
Consider the following code:
const AsyncLock = require('async-lock');
const lock = new AsyncLock;
lock.acquire("key", function (done) {
throw new Error("error");
});
I would expect that acquire()
would return rejected Promise but I got Uncaught Error: error
, and the lock is not released.
The snippet is just slightly different from the one in the README which returns rejected Promise.
lock.acquire(key, function() {
throw new Error('error');
})
From your point of view, is my code just improper usage of the library or can we consider it as a bug? If so, I'd be happy to try to prepare PR.
When passing in options to acquire
, maxPending
is not respected and the function still uses the default of 1000.
Currently files are only readable by owner, this leads to issues in production environments where the ownership of the files are not always the same as the user executing nodejs
.
Actual results:
curl -s "https://registry.npmjs.org/async-lock/-/async-lock-1.1.2.tgz" | tar -tvz
-rw-r--r-- 0/0 1140 2018-02-27 03:19 package/package.json
-rw------- 0/0 321 2018-02-27 03:19 package/AUTHORS
-rw------- 0/0 1595 2018-02-27 03:19 package/History.md
-rw------- 0/0 49 2018-02-27 03:19 package/index.js
-rw------- 0/0 1110 2018-02-27 03:19 package/LICENSE
-rw-r--r-- 0/0 4279 2018-02-27 03:19 package/README.md
-rw-r--r-- 0/0 5992 2018-02-27 03:20 package/lib/index.js
Expected results:
curl -s "https://registry.npmjs.org/async-lock/-/async-lock-1.1.2.tgz" | tar -tvz
-rw-r--r-- 0/0 1140 2018-02-27 03:19 package/package.json
-rw-r--r-- 0/0 321 2018-02-27 03:19 package/AUTHORS
-rw-r--r-- 0/0 1595 2018-02-27 03:19 package/History.md
-rw-r--r-- 0/0 49 2018-02-27 03:19 package/index.js
-rw-r--r-- 0/0 1110 2018-02-27 03:19 package/LICENSE
-rw-r--r-- 0/0 4279 2018-02-27 03:19 package/README.md
-rw-r--r-- 0/0 5992 2018-02-27 03:20 package/lib/index.js
Just curious, how long does a lock last?
Things like Promise.try
, .nodeify
, and . defer
on non-standard and it would be nice if they were not relied on.
Hello, first of all, thanks for this module.
I was wondering if there is any way to empty the queue of pending locks. At some point, I don't care anymore about what is pending so I would like to cancel everything that is pending in the queue.
Thanks :)
maxPending
can be Inifinity in isomorphic-git, for example. Would be great to keep support of it. One of possible solutions can be changing Number.isInteger into !Number.isNaN
.
this.maxPending = !Number.isNaN(opts.maxPending) ? opts.maxPending : AsyncLock.DEFAULT_MAX_PENDING;
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.