1

In the following class, the _busy field acts as a semaphore; but, in "simultaneous" situations it fails to guard!

class Task { _busy = false; async run(s) { try { if (this._busy) return; this._busy = true; await payload(); } finally { this._busy = false; } } } 

The sole purpose of the run() is to execute the payload() exclusively, denying all the other invocations while it's still being carried on. In other words, when "any" of the invocations reach to to the run() method, I want it to only allow the first one to go through and lock it down (denying all the others) until it's done with its payload; "finally", it opens up once it's done.

In the implementation above, the racing condition do occur by invoking the run() method simultaneously through various parts of the app. Some of the invocations (more than 1) make it past through the "guarding" if statement, since none of them are yet reached to the this._busy = true to lock it down (they get past simultaneously). So, the current implementation doesn't cut it!

I just want to deny the simultaneous invocations while one of them is already being carried out. I'm looking for a simple solution to only resolve this issue. I've designated the async-mutex library as a last resort!

So, how to implement a simple "locking" mechanism to avoid racing conditions that bypass the guard statement in simultaneous actions?


For more clarification, as per the comments below, the following is almost the actual Task class (without the irrelevant).

class Task { _cb; _busy = false; _count = 0; constructor(cb) { this._cb = cb; } async run(params = []) { try { if (this._busy) return; this._busy = true; this._count++; if (this._count > 1) { console.log('Race condition!', 'count:', this._count); this._count--; return; } await this._cb(...params); } catch (err) { await someLoggingRoutine(); } finally { this._busy = false; this._count--; } } } 

I do encounter with the Race condition! log. Also, all the task instances are local to a simple driver file (the instances are not passed down to any other function, they only wander as local instances in a single function.) They are created in the following form:

const t1 = new Task(async () => { await doSth1(); /*...*/ }); const t2 = new Task(async () => { await doSth2(); /*...*/ }); const t3 = new Task(async () => { await doSth3(); /*...*/ }); // ... 

I do call them in the various library events, some of which happen concurrently and causing the "race condition" issue; e.g.:

someLib.on('some-event', async function() { /*...*/ t1.run().then(); /*...*/ }); anotherLib.on('event-2', async function() { /*...*/ t1.run().then(); /*...*/ }); 
10
  • Unless you are using worker threads, there is no multithreading in javascript and your described scenario can't happen. Commented Nov 28, 2022 at 16:40
  • I use some libraries (like database and other network libraries) that do use worker threads internally. I do encounter with racing condition in my actual app! Commented Nov 28, 2022 at 16:56
  • I actually find that hard to believe for various reasons. Are you entirely sure, all your different invocations are using the very same instance of your Task class? Because, of course each new instance of the Task class will allow one additional "parallel" execution of the payload() Commented Nov 28, 2022 at 17:05
  • Even if some of your used libraries use worker thread internally, either those libraries would be needed to run your tasks on those workerthreads, or you yourself must create multiple threads to run your tasks. And even then it seems very unlikely (eventough not impossible at a certain workload) to me, that two workerthreads are running so simulantiously that your described scenario would happen. Let alone the problem of shared memory between workerthreads which isn't so easy to achieve ... Commented Nov 28, 2022 at 17:10
  • I'm absolutely sure that a same instance of the Task is involved and there's no misunderstanding here (the respective code is very small and the certainty is easily conformable by a simple eyeballing!) Also, the payload() here is just a placeholder. The "payload" is actually a callback function assigned in the constructor in the actual class (the provided code is just a simplification to convey the point I'm trying to say.) Commented Nov 28, 2022 at 17:32

1 Answer 1

1

Oh god, now I see it. How could I have missed this so long! Here is your implemenation:

async run() { try { if (this._busy) return; ... } finally { this._busy = false; } } 

As per documentations:

The Statements in the finally block are executed before control flow exits the try...catch...finally construct. These statements execute regardless of whether an exception was thrown or caught.

Thus, when it's busy and the flow reaches the guarding if, and then, logically encounters the return statement. The return statement causes the flow to exit the try...catch...finally construct; thus, as per the documentations, the statements in the finally block are executed whatsoever: setting the this._busy = false;, opening the thing up!


So, the first call of run() sets this._busy as being true; then enters the critical section with its longrunning callback. While this callback is running, just another event causes the run() to be invoked. This second call is rationally blocked from entering the critical section by the guarding if statement:

if (this._busy) return; 

Encountering the return statement to exit the function (and thus exiting the try...catch...finally construct) causes the statements in the finally block to be executed; thus, this._busy = false resets the flag, even though the first callback is still running! Now suppose a third call to the run() from yet another event is invoked! Since this._busy is just set to false, the flow happily enters the critical section again, even though the first callback is still running! In turn, it sets this._busy back to true. In the meantime, the first callback finishes, and reaches the finally block, where it set this._busy = false again; even though the other callback is still running. So the next call to run() can enter the critical section again with no problems... And so on and so forth...

So to resolve the issue, the check for the critical section should be outside of the try block:

async run() { if (this._busy) return; this._busy = true; try { ... } finally { this._busy = false; } } 
Sign up to request clarification or add additional context in comments.

1 Comment

This is genius! You've nailed it; I truly appreciate it, Thank you.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.