search

breejs / bree

Bree is a Node.js and JavaScript job task scheduler with worker threads, cron, Date, and human syntax. Built for @ladjs, @forwardemail, @spamscanner, @cabinjs.
https://jobscheduler.net
MIT License
3.01k stars 78 forks source link

[fix] properly handle closeWorkerAfterMs timeout #130

Closed climba03003 closed 2 years ago

climba03003 commented 2 years ago

Currently, closeWorkerAfterMs is created without clear for each online event. It leads to a problem that the old timeout will clear a newly created worker process and it may throw a exit code 1 in this case.

For example, a job with interval 1 min and closeWorkerAfterMs 2 mins. The second run of job will terminate by the first closeWorkerAfterMs timeout randomly.

How to solve

We can use .clear before the worker is created. https://github.com/breejs/bree/blob/769c6bddb879a9eaf6b08e5a0e3f77fd9d06c53a/src/index.js#L257-L271

Reason

Why we .clear on before it create but not worker deleted ? Because it may accidentally remove the wrong timeout.

The worker will be started only if the old worker is closed and removed. It should ensure the old timeout must be cleared.

shadowgate15 commented 2 years ago

good catch. I would say that should be deleted when done is sent or on exit. if the worker still existed the job will never be run.

this could also be a performance improvement. since it would also remove any hanging intervals without them firing when they don't need to.