enten / udk

Universal Development Kit: Webpack extension which improves universal application development. - THE UDK PROJECT SUPPORT IS CURRENTLY SUSPENDED.
MIT License
29 stars 7 forks source link

WatchpackFork: killer's callback called twice #1

Closed enten closed 5 years ago

enten commented 6 years ago

In lib/util/WatchpackFork.js, the method close() used the package killer to kill the child process.

On Windows, the callback passed to killer is called more than once: that is a source of bugs.

// udk/lib/util/WatchpackFork
// 0d5d7da on 19 Oct 2017@enten

const Watchpack2 = require('./Watchpack2')
const childProcess = require('child_process')
const killer = require('killer')

class WatchpackFork extends Watchpack2 {
  // ...
  close (callback) {
    const done = () => {
      this.child = undefined

      callback && callback()
    }

    return super.close(() => {
      if (!this.child) {
        return done()
      }

      killer(this.child.pid, done)
    })
  }
  // ...
}

For example, in lib/util/container.js, the method runContainer() call the method closeContainer() when the container has already a watcher (if the container is already forked): that method is called with a callback which will be called by WatchpackFork.close() when the fork process will be killed. That callback will restart the container: if it called more than once, each called will potentially starts a new fork process.

Temporary fix: we can ensure that the callback is called only once.

  close (callback) {
    let doneCalled = false // +++

    const done = () => {
      if (doneCalled) { // +++
        return // +++
      } // +++

      doneCalled = true // +++
      this.child = undefined

      callback && callback()
    }

    return super.close(() => {
      if (!this.child) {
        return done()
      }

      killer(this.child.pid, done)
    })
  }

Better fix: understand why killer called the callback twice under Windows and fix it.

TODO

enten commented 5 years ago

Close because WatchpackFork doesn't exist anymore.

The issue was due to the design of the MultCompiler2 (v0.3.x) which freeze compilation in compiler hook. Now, we freeze compilation by hook Compiler.compile method.