Let's add a closed property to child process and use the following logic:

ChildProcess.prototype[SymbolAsyncDispose] = async function() {
  if (!this.closed) {
    await EE.once(this, 'close');
  }
};

I'm unsure whether it's sufficient to wait for 'exit' or 'close' is the more correct event to wait for? Not sure exactly what the difference is between these two.

according to the code, exit is emitted first when child process exits, then close is fired once all the streams are drained. so we should probably use close

Let's add a closed property to child process and use the following logic:

ChildProcess.prototype[SymbolAsyncDispose] = async function() {
  if (!this.closed) {
    await EE.once(this, 'close');
  }
};

we cannot use EE.once since it will always reject when calling abortChildProcess.

Any signal other than SIGKILL may actually be ignored by the process. Which would lead to a deadlock in our case.

Any signal other than SIGKILL may actually be ignored by the process. Which would lead to a deadlock in our case.

a pending promise is not a deadlock

It is if you wait for it, expecting it to resolve and it never does

I'd like to have some more opinions on this. I feel there is a risk of this being an anti-pattern that we encourage.

@benjamingr @jasnell

Maybe we can get around the deadlock issue by always having a timeout which eventually calls sigkill and makes sure the promise will eventually resolve.

An alternative is to implement dispose and not asyncDispose, send the signal and wait for nothing. If someone wants to wait for the process to terminate they can do so by listening to the event themselves.

An alternative is to implement dispose and not asyncDispose, send the signal and wait for nothing. If someone wants to wait for the process to terminate they can do so by listening to the event themselves.

I think this is the only reasonable option given we don't know how to wait for the process to terminate "for sure"

CI: https://ci.nodejs.org/job/node-test-pull-request/52610/

CI: https://ci.nodejs.org/job/node-test-pull-request/52613/

Landed in 773fde2