maxOccupationTime not working as I would expect

[joeschneider32]

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
  })
}

[rogierschouten]

Hi, it's hard to know what the original author meant to do - given the comment

// occupation time exceeded error will be returned here if job not completed in given time

I could take it to mean either queue + execution time, or just execution time.

Looking at the code, it looks really straightforward: as soon as you call acquire, the timer is started. The timeout setting seems to pertain to the queue only, and occupation timer to queue+execution.

I agree that this is not very sensible, however I am afraid that if we change this, it would cause compatibility issues.

As you may be aware already, I don't maintain this other than merging PRs (since I don't code anymore and have no use for this anymore). I can imagine at least clarifying the README.md; maybe introducing a third option 'executiontimeout'.

My bigger question is: why would you need an execution timeout? It's not like you can stop the execution. And you can easily create an execution timeout already, just start it in the body of your function and make sure it stops whatever you are doing. Much more robust design as well...

[joeschneider32]

Hi @rogierschouten ,

Thanks for your response. My initial thought when using this option was to be extra cautious and avoid the lock being held forever somehow but you are right there are more straight forward ways to implement this myself.

Thanks for your help!

[rogierschouten]

Thx @joeschneider32 for the pull request!