Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled ERR_IPC_CHANNEL_CLOSED exception and a deadlock in jest-worker #11143

Closed
xamgore opened this issue Mar 3, 2021 · 4 comments · Fixed by #11144
Closed

Unhandled ERR_IPC_CHANNEL_CLOSED exception and a deadlock in jest-worker #11143

xamgore opened this issue Mar 3, 2021 · 4 comments · Fixed by #11144

Comments

@xamgore
Copy link
Contributor

xamgore commented Mar 3, 2021

An unhandled ERR_IPC_CHANNEL_CLOSED exception can be thrown while calling await worker.end(). Follow <<<<<<<<<<<<<<<<<<<<<<<< in the code:

  // class JestWorker
  async end() {
    if (this._ending) {
      throw new Error('Farm is ended, no more calls can be done to it');
    }

    this._ending = true;
    return this._workerPool.end(); <<<<<<<<<<<<<<<<<<<<<<<<
  }
 // BaseWorkerPool.js
 async end() {
    // We do not cache the request object here. If so, it would only be only
    // processed by one of the workers, and we want them all to close.
    const workerExitPromises = this._workers.map(async worker => {
      worker.send( <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
        [_types().CHILD_MESSAGE_END, false],
        emptyMethod,
        emptyMethod,
        emptyMethod
      );
      // Schedule a force exit in case worker fails to exit gracefully so
      // await worker.waitForExit() never takes longer than FORCE_EXIT_DELAY

      let forceExited = false;
      const forceExitTimeout = setTimeout(() => {
        worker.forceExit();
        forceExited = true;
      }, FORCE_EXIT_DELAY);
      await worker.waitForExit(); // Worker ideally exited gracefully, don't send force exit then

      clearTimeout(forceExitTimeout);
      return forceExited;
    });

    const workerExits = await Promise.all(workerExitPromises);
    return workerExits.reduce(
      (result, forceExited) => ({
        forceExited: result.forceExited || forceExited
      }),
      {
        forceExited: false
      }
    );
  }
  // ChildProcessWorker.js
  send(request, onProcessStart, onProcessEnd, onCustomMessage) {
    onProcessStart(this);

    this._onProcessEnd = (...args) => {
      // Clean the request to avoid sending past requests to workers that fail
      // while waiting for a new request (timers, unhandled rejections...)
      this._request = null;
      return onProcessEnd(...args);
    };

    this._onCustomMessage = (...arg) => onCustomMessage(...arg);

    this._request = request;
    this._retries = 0;

    this._child.send(request); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  }
  // core_modules/internal/child_process.js
  target.send = function(message, handle, options, callback) {
    if (typeof handle === 'function') {
      callback = handle;
      handle = undefined;
      options = undefined;
    } else if (typeof options === 'function') {
      callback = options;
      options = undefined;
    } else if (options !== undefined &&
               (options === null || typeof options !== 'object')) {
      throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
    }

    options = { swallowErrors: false, ...options };

    if (this.connected) {
      return this._send(message, handle, options, callback);
    }
    const ex = new ERR_IPC_CHANNEL_CLOSED(); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    if (typeof callback === 'function') {
      process.nextTick(callback, ex);
    } else {
      process.nextTick(() => this.emit('error', ex));
    }
    return false;
  };

The target.send function emits an error when the callback is not passed via the second argument. Jest calls it without any callback: this._child.send(request);. The error claims:

An attempt was made to use an IPC communication channel that was already closed.

Ok, that can be fixed by passing console.error or () => {}. Let's get back to BaseWorkerPool.js. We call then forceExit() method and start awaiting for worker.waitForExit();.

  this._exitPromise = new Promise(resolve => {
    this._resolveExitPromise = resolve;
  });

  waitForExit() {
    return this._exitPromise;
  }

  forceExit() {
    this._child.kill('SIGTERM'); // calls this._resolveExitPromise() indirectly through _onExit

    const sigkillTimeout = setTimeout(
      () => this._child.kill('SIGKILL'),
      SIGKILL_DELAY
    );

    this._exitPromise.then(() => clearTimeout(sigkillTimeout));
  }

At this point, I observe an everlasting program execution. Let's assume this._exitPromise is never resolved, that is because this._resolveExitPromise is never called. The only place where it is called is _onExit(exitCode):

_onExit(exitCode) { console.error('_onExit', exitCode) if ( exitCode !== 0 && exitCode !== SIGTERM_EXIT_CODE && exitCode !== SIGKILL_EXIT_CODE ) { this.initialize();
  if (this._request) {
    this._child.send(this._request);
  }
} else {
  this._shutdown(); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
}

}

_shutdown() {
// End the temporary streams so the merged streams end too
if (this._fakeStream) {
this._fakeStream.end();

  this._fakeStream = null;
}

this._resolveExitPromise(); <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

}

What if _onExit is called with the null argument? Node.js documentation assures such cases, so we need exitCode !== null clause.

@xamgore
Copy link
Contributor Author

xamgore commented Mar 6, 2021

@SimenB could you be so kind to have a look? I want to be sure I properly grasped the problem and the fix is correct.

@SimenB
Copy link
Member

SimenB commented Mar 6, 2021

This looks good to me! @jeysal thoughts?

@jeysal
Copy link
Contributor

jeysal commented Mar 6, 2021

Yeah, seems good. Probably almost impossible to write a regression test for? So might have to merge it and hope no one breaks it in the future 🙈

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants