samchon/tstl

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:

if (prev.equals(this.queue_.end()) === false && prev.value.handler === null)
this._Release(1);

Therefore, I've patched the tstl to 2.4.8 to fix the code like below:

if (prev.equals(this.queue_.end()) === false && prev.value.handler !== null)
this._Release(1);

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