fix(cron): fix 2 race conditions (#4533)

Should fix xoa-support#344, xoa-support#1186 & xoa-support#1755.

These could lead to:
- job not properly stopped
- job run twice

@xen-orchestra/cron/src/index.js

@@ -13,6 +13,8 @@ function nextDelay(schedule) {
 class Job {
   constructor(schedule, fn) {
     const wrapper = () => {
+      this._isRunning = true
+
       let result
       try {
         result = fn()
@@ -27,23 +29,34 @@ class Job {
       }
     }
     const scheduleNext = () => {
-      const delay = nextDelay(schedule)
-      this._timeout =
-        delay < MAX_DELAY
-          ? setTimeout(wrapper, delay)
-          : setTimeout(scheduleNext, MAX_DELAY)
+      this._isRunning = false
+
+      if (this._isEnabled) {
+        const delay = nextDelay(schedule)
+        this._timeout =
+          delay < MAX_DELAY
+            ? setTimeout(wrapper, delay)
+            : setTimeout(scheduleNext, MAX_DELAY)
+      }
     }
 
+    this._isEnabled = false
+    this._isRunning = false
     this._scheduleNext = scheduleNext
     this._timeout = undefined
   }
 
   start() {
     this.stop()
-    this._scheduleNext()
+
+    this._isEnabled = true
+    if (!this._isRunning) {
+      this._scheduleNext()
+    }
   }
 
   stop() {
+    this._isEnabled = false
     clearTimeout(this._timeout)
   }
 }

@xen-orchestra/cron/src/index.spec.js

@@ -0,0 +1,62 @@
+/* eslint-env jest */
+
+import { createSchedule } from './'
+
+describe('issues', () => {
+  test('stop during async execution', async () => {
+    let nCalls = 0
+    let resolve, promise
+
+    const job = createSchedule('* * * * *').createJob(() => {
+      ++nCalls
+
+      // eslint-disable-next-line promise/param-names
+      promise = new Promise(r => {
+        resolve = r
+      })
+      return promise
+    })
+
+    job.start()
+    jest.runAllTimers()
+
+    expect(nCalls).toBe(1)
+
+    job.stop()
+
+    resolve()
+    await promise
+
+    jest.runAllTimers()
+    expect(nCalls).toBe(1)
+  })
+
+  test('stop then start during async job execution', async () => {
+    let nCalls = 0
+    let resolve, promise
+
+    const job = createSchedule('* * * * *').createJob(() => {
+      ++nCalls
+
+      // eslint-disable-next-line promise/param-names
+      promise = new Promise(r => {
+        resolve = r
+      })
+      return promise
+    })
+
+    job.start()
+    jest.runAllTimers()
+
+    expect(nCalls).toBe(1)
+
+    job.stop()
+    job.start()
+
+    resolve()
+    await promise
+
+    jest.runAllTimers()
+    expect(nCalls).toBe(2)
+  })
+})

CHANGELOG.unreleased.md

@@ -21,6 +21,7 @@
 - [Backup NG/logs] Show warning when zstd compression is selected but not supported [#3892](https://github.com/vatesfr/xen-orchestra/issues/3892) (PR [#4375](https://github.com/vatesfr/xen-orchestra/pull/4375)
 - [Patches] Fix patches installation for CH 8.0 (PR [#4511](https://github.com/vatesfr/xen-orchestra/pull/4511))
 - [Network] Fix inability to set a network name [#4514](https://github.com/vatesfr/xen-orchestra/issues/4514) (PR [4510](https://github.com/vatesfr/xen-orchestra/pull/4510))
+- [Backup NG] Fix race conditions that could lead to disabled jobs still running (PR [4510](https://github.com/vatesfr/xen-orchestra/pull/4510))
 
 ### Released packages
 
@@ -29,6 +30,7 @@
 >
 > Rule of thumb: add packages on top.
 
+- @xen-orchestra/cron v1.0.4
 - xo-server-sdn-controller v0.3.0
 - xo-server v5.50.0
 - xo-web v5.50.0

package.json

@@ -46,6 +46,7 @@
       "/xo-web/"
     ],
     "testRegex": "\\.spec\\.js$",
+    "timers": "fake",
     "transform": {
       "\\.jsx?$": "babel-jest"
     }