fix: Prevent Chokidar events processing issues (#2325)

.github/actions/setup-dnsmasq/action.yaml

@@ -0,0 +1,23 @@
+name: Setup cozy-stack
+author: Erwan Guyader
+description: Setup dnsmasq for .localhost domains on macOS
+runs:
+  using: composite
+  steps:
+    - name: Install dnsmasq
+      shell: bash
+      run: brew install dnsmasq
+
+    - name: Add address entry to point .localhost to 127.0.0.1
+      shell: bash
+      run: echo "address=/.localhost/127.0.0.1" >> "$(brew --prefix)"/etc/dnsmasq.conf
+
+    - name: Start dnsmasq
+      shell: bash
+      run: sudo brew services start dnsmasq
+
+    - name: Create resolver configuration
+      shell: bash
+      run: |
+        sudo mkdir -p /etc/resolver
+        echo "nameserver 127.0.0.1" | sudo tee /etc/resolver/localhost

.github/workflows/macos.yaml

@@ -94,6 +94,8 @@ jobs:
         uses: ./.github/actions/setup-cozy-stack
         with:
           couchdb-url: ${{ steps.setup-couchdb.outputs.couchdb-url }}
+      - name: Setup DNS resolution for .localhost
+        uses: ./.github/actions/setup-dnsmasq
       - name: Setup local env
         env:
           COZY_DESKTOP_FS: ${{ matrix.fs }}
@@ -151,6 +153,8 @@ jobs:
         uses: ./.github/actions/setup-cozy-stack
         with:
           couchdb-url: ${{ steps.setup-couchdb.outputs.couchdb-url }}
+      - name: Setup DNS resolution for .localhost
+        uses: ./.github/actions/setup-dnsmasq
       - name: Setup local env
         env:
           COZY_DESKTOP_FS: ${{ matrix.fs }}
@@ -209,6 +213,8 @@ jobs:
         uses: ./.github/actions/setup-cozy-stack
         with:
           couchdb-url: ${{ steps.setup-couchdb.outputs.couchdb-url }}
+      - name: Setup DNS resolution for .localhost
+        uses: ./.github/actions/setup-dnsmasq
       - name: Setup local env
         env:
           COZY_DESKTOP_FS: ${{ matrix.fs }}

core/local/chokidar/analysis.js

@@ -60,13 +60,13 @@ module.exports = function analysis(
   {
     pendingChanges,
     initialScanParams
-  } /*: { pendingChanges: LocalChange[], initialScanParams: ?InitialScanParams } */
+  } /*: { pendingChanges: LocalChange[], initialScanParams: InitialScanParams } */
 ) /*: LocalChange[] */ {
   const changes /*: LocalChange[] */ = analyseEvents(events, pendingChanges)
   fixUnsyncedMoves(changes)
   sortBeforeSquash(changes)
   squashMoves(changes)
-  sortChanges(changes, initialScanParams != null)
+  sortChanges(changes, !initialScanParams.done)
   return separatePendingChanges(changes, pendingChanges)
 }
 

core/local/chokidar/event_buffer.js

@@ -27,6 +27,7 @@ type FlushCallback<EventType> = (EventType[]) => any
  */
 class EventBuffer /*:: <EventType> */ {
   /*::
+  locked: boolean
   events: EventType[]
   mode: EventBufferMode
   timeoutInMs: number
@@ -38,6 +39,7 @@ class EventBuffer /*:: <EventType> */ {
     timeoutInMs /*: number */,
     flushed /*: FlushCallback<EventType> */
   ) {
+    this.locked = false
     this.events = []
     this.mode = 'idle'
     this.timeoutInMs = timeoutInMs
@@ -47,6 +49,18 @@ class EventBuffer /*:: <EventType> */ {
     autoBind(this)
   }
 
+  lock() {
+    if (this.locked) {
+      throw new Error('lock unavailable')
+    }
+
+    this.locked = true
+  }
+
+  unlock() {
+    this.locked = false
+  }
+
   push(event /*: EventType */) /*: void */ {
     this.events.push(event)
     this.shiftTimeout()
@@ -72,11 +86,22 @@ class EventBuffer /*:: <EventType> */ {
   }
 
   async flush() {
-    this.clearTimeout()
-    if (this.events.length > 0) {
-      const flushedEvents = this.events
-      this.events = []
-      return this.flushed(flushedEvents)
+    try {
+      this.lock()
+    } catch (err) {
+      this.shiftTimeout()
+      return
+    }
+
+    try {
+      this.clearTimeout()
+      if (this.events.length > 0) {
+        const flushedEvents = this.events
+        this.events = []
+        await this.flushed(flushedEvents)
+      }
+    } finally {
+      this.unlock()
     }
   }
 

core/local/chokidar/initial_scan.js

@@ -25,7 +25,8 @@ export type InitialScanParams = {
   paths: string[],
   emptyDirRetryCount: number,
   flushed: boolean,
-  resolve: () => void
+  done: boolean,
+  resolve?: () => void
 }
 
 export type InitialScanOpts = {
@@ -68,9 +69,10 @@ const step = async (
   rawEvents /*: ChokidarEvent[] */,
   { buffer, initialScanParams, pouch } /*: InitialScanOpts */
 ) /*: Promise<Array<ChokidarEvent>> */ => {
-  // We mark the initial scan as flushed as soon as possible to avoid
-  // concurrent initial scan processings from later flushes.
+  // We mark the initial scan as flushed as soon as possible so latter events
+  // are not marked as part of the initial scan.
   initialScanParams.flushed = true
+
   let events = rawEvents
 
   events

core/local/chokidar/prepare_events.js

@@ -39,7 +39,7 @@ import type { Pouch } from '../../pouch'
 
 type PrepareEventsOpts = {
   +checksum: (string) => Promise<string>,
-  initialScanParams: ?InitialScanParams,
+  initialScanParams: InitialScanParams,
   pouch: Pouch,
   syncPath: string
 }

core/local/chokidar/watcher.js

@@ -73,7 +73,7 @@ class LocalWatcher {
   prep: Prep
   pouch: Pouch
   events: EventEmitter
-  initialScanParams: ?InitialScanParams
+  initialScanParams: InitialScanParams
   checksumer: Checksumer
   watcher: any // chokidar
   buffer: LocalEventBuffer<ChokidarEvent>
@@ -93,6 +93,16 @@ class LocalWatcher {
     this.checksumer = checksumer.init()
     this.pendingChanges = []
 
+    // To detect which files&folders have been removed since the last run of
+    // cozy-desktop, we keep all the paths seen by chokidar during its
+    // initial scan in @paths to compare them with pouchdb database.
+    this.initialScanParams = {
+      paths: [],
+      emptyDirRetryCount: 3,
+      flushed: false,
+      done: false
+    }
+
     // XXX: this.onFlush must be bound before being passed to LocalEventBuffer
     autoBind(this)
 
@@ -115,6 +125,8 @@ class LocalWatcher {
   start() {
     log.debug('Starting...')
 
+    this.resetInitialScanParams()
+
     this.watcher = chokidar.watch('.', {
       // Let paths in events be relative to this base path
       cwd: this.syncPath,
@@ -140,6 +152,8 @@ class LocalWatcher {
     })
 
     const started = new Promise(resolve => {
+      this.initialScanParams.resolve = resolve
+
       for (let eventType of [
         'add',
         'addDir',
@@ -150,8 +164,7 @@ class LocalWatcher {
         this.watcher.on(
           eventType,
           (path /*: ?string */, stats /*: ?fs.Stats */) => {
-            const isInitialScan =
-              this.initialScanParams && !this.initialScanParams.flushed
+            const isInitialScan = !this.initialScanParams.flushed
             log.chokidar.debug({ path, stats, isInitialScan }, eventType)
             const newEvent = chokidarEvent.build(eventType, path, stats)
             if (newEvent.type !== eventType) {
@@ -166,16 +179,6 @@ class LocalWatcher {
         )
       }
 
-      // To detect which files&folders have been removed since the last run of
-      // cozy-desktop, we keep all the paths seen by chokidar during its
-      // initial scan in @paths to compare them with pouchdb database.
-      this.initialScanParams = {
-        paths: [],
-        emptyDirRetryCount: 3,
-        resolve,
-        flushed: false
-      }
-
       this.watcher
         .on('ready', () => this.buffer.switchMode('timeout'))
         .on('raw', async (event, path, details) => {
@@ -219,20 +222,14 @@ class LocalWatcher {
     this.events.emit('local-start')
 
     let events = rawEvents.filter(hasPath) // @TODO handle root dir events
-    // We need to destructure `this` otherwise Flow won't detect that
-    // `this.initialScanParams` is not null even within the conditional block.
-    const { buffer, pouch, initialScanParams } = this
-    if (initialScanParams != null && !initialScanParams.flushed) {
-      events = await initialScan.step(events, {
-        initialScanParams,
-        buffer,
-        pouch
-      })
+
+    if (!this.initialScanParams.flushed) {
+      events = await initialScan.step(events, this)
     }
 
     if (events.length === 0) {
       this.events.emit('local-end')
-      if (this.initialScanParams != null) this.initialScanParams.resolve()
+      this.endInitialScan()
       return
     }
 
@@ -263,18 +260,16 @@ class LocalWatcher {
       release()
       this.events.emit('local-end')
     }
-    if (this.initialScanParams != null) {
-      this.initialScanParams.resolve()
-      this.initialScanParams = null
-    }
+
+    this.endInitialScan()
   }
 
   async stop(force /*: ?bool */ = false) {
     log.debug('Stopping watcher...')
 
     if (!this.watcher) return
 
-    if (force) {
+    if (force || !this.initialScanParams.flushed) {
       // Drop buffered events
       this.buffer.clear()
     } else {
@@ -294,7 +289,7 @@ class LocalWatcher {
     // Stop underlying Chokidar watcher
     await this.watcher.close()
     this.watcher = null
-    // Stop accepting new events
+    // Flush buffer and stop flushes loop
     this.buffer.switchMode('idle')
 
     if (!force) {
@@ -305,6 +300,22 @@ class LocalWatcher {
     }
   }
 
+  resetInitialScanParams() {
+    this.initialScanParams = {
+      paths: [],
+      emptyDirRetryCount: 3,
+      flushed: false,
+      done: false
+    }
+  }
+
+  endInitialScan() {
+    if (this.initialScanParams.resolve) {
+      this.initialScanParams.done = true
+      this.initialScanParams.resolve()
+    }
+  }
+
   /* Helpers */
   async checksum(filePath /*: string */) /*: Promise<string> */ {
     const absPath = path.join(this.syncPath, filePath)

test/scenarios/run.js

@@ -282,6 +282,8 @@ async function runLocalChokidarWithCaptures(
       helpers.local.syncDir.abspath,
       eventsFile
     )
+    // XXX: Run initial scan
+    await helpers.local.scan()
   }
 
   const inodeChanges = await runActions(

test/support/helpers/pouch.js

@@ -9,7 +9,16 @@ module.exports = {
 
   async cleanDatabase() {
     if (this.pouch && this.pouch.db) {
-      await this.pouch.db.destroy()
+      let retry = 0
+      do {
+        try {
+          await this.pouch.db.destroy()
+          break
+        } catch (err) {
+          retry++
+          await Promise.delay(500 * 2 ** retry)
+        }
+      } while (retry < 3)
     }
     this.pouch = null
   },