it seems that Semaphore try_acquire_for has some bug
chacent opened this issue · 12 comments
it occors multiple times ,at last the bug always(at least two diff scene) found at try_acquire_for and release,,but i can't repeat it with simply code. i believe that it has a bug
I'm going to review implementation of the Semaphore
class in this weekend, may Sunday. However, it would be better you to provide me more detailed information or some of the test code who can reproduce the bug.
the counter seems going bad when lots of try_acquire_for,at last semaphore can't be acquired. i try to make the scene but i failed,It's not easy to reproduce
i'm sure that my code has release every time,and the function acquire do well always
my code is like this
(async()=>{
while(true){
if(await s.try_acquire_for(1000)){
(async()=>{
const wait = Math.ceil(Math.random()*10);
console.log(1,wait);
await new Promise(s=>setTimeout(s,wait));
})().finally(()=>s.release());
}
}
})();
This is the code that i actually used
thread_middleware() {
return async(ctx,next)=>{
if(await this[$semaphore].try_acquire_for(1000)){
try{
await next();
}finally {
this[$semaphore].release();
}
}else{
throw new Error('server busy');
}
}
}
sorry for that i don't know how to format the code
In tstl@2.4.7
, I've tried your 1st code like below and it seems no error.
import { Semaphore, randint, sleep_for } from "tstl";
async function main(): Promise<void>
{
let s: Semaphore = new Semaphore(4);
let minimum: number = 500;
let maximum: number = 4000;
while(true)
{
let waitTime: number = Date.now();
let success: boolean = await s.try_acquire_for(1000);
waitTime = Date.now() - waitTime;
if (success === true)
{
(async () =>
{
let sleepTime: number = randint(minimum, maximum);
console.log("success, sleep_for", sleepTime, "ms after", waitTime, "ms");
await sleep_for(sleepTime);
await s.release();
})();
}
else
console.log("failed after", waitTime, "ms");
}
}
main();
success, sleep_for 2490 ms after 1 ms success, sleep_for 1140 ms after 0 ms success, sleep_for 3132 ms after 0 ms success, sleep_for 1355 ms after 0 ms failed after 1011 ms success, sleep_for 1760 ms after 139 ms success, sleep_for 3712 ms after 217 ms failed after 1011 ms success, sleep_for 2664 ms after 108 ms success, sleep_for 3819 ms after 421 ms success, sleep_for 3290 ms after 231 ms failed after 1007 ms success, sleep_for 3995 ms after 931 ms success, sleep_for 3465 ms after 76 ms failed after 1009 ms success, sleep_for 2902 ms after 263 ms success, sleep_for 3782 ms after 294 ms failed after 1008 ms success, sleep_for 3677 ms after 900 ms success, sleep_for 2255 ms after 450 ms success, sleep_for 867 ms after 246 ms success, sleep_for 3709 ms after 870 ms success, sleep_for 2970 ms after 308 ms
yes,i found no error when test,and this is my test code.but when actual project it occors.now i use acquire instead. i will focus with this later
I'd found a mistaken code that can occur a runtime error in the Semaphore._Cancel()
method:
Lines 145 to 146 in 228e247
Therefore, I've patched the tstl to 2.4.8
to fix the code like below:
Lines 140 to 141 in 4be6620
Anyway, looking at your 2nd code, it seems @chacent, you're using the nestjs
framework, right? I'll test similar code by creating a test web-server project using the nestjs
at next weekend, may Saturday. Until the next wekkend, I hope @chacent you to upgrade your project to using the new tstl@2.4.8
and tell me if same error occurs.
Thank you. I'm sure this problem has no relation with the library I used. If I have time to try to find another piece of code that I made a mistake last time, it must be a problem of counting logic, which may be caused by reentry
and maybe your patch code works, i'll try tomorrow
my framework used is koa,just ignore it,thanks