Worker process not terminated when Babel emits ReferenceError on malformed options

[Elberet]

Consider this snippet based on broccoli-test-helper:

let tree = new babel("src", { unknownOption: true })
let output = createBuilder(tree)
try {
  await output.build()
} finally {
  await output.dispose()
}

When the tree is processed, Babel will eventually be handed an options object with an unknownOption key and will (rightfully) raise a ReferenceError. So far, no surprises.

As a result, however, the worker process where the error was raised doesn't get terminated even after the Broccoli builder has been cleaned up; the parent Node process will not terminate once work has been exhausted.

My guess is that this is caused by workerpool and not this package, but I'm not familiar enough with that to know whether it behaves as documented or whether it's bugged. On the other hand, it should be fairly simple to implement a workaround by catching errors in lib/worker.js, resolving the promise with the caught error and re-throwing it in lib/parallel-api.js → transformString.

[Turbo87]

/cc @stefanpenner

[stefanpenner]

As a result, however, the worker process where the error was raised doesn't get terminated even after the Broccoli builder has been cleaned up; the parent Node process will not terminate once work has been exhausted

Ah this isn’t good, I’ll take a look.
It’s worth pointing out, if you are on the latest broccoli-Babel-transpiler and on node 11. It will use WorkerThreads instead of processes, so it should address this problem (at least as a workaround and a good future solution, but we should still obviously fix the issue you describe). Mind giving that a try?

[Elberet]

Can do, but it'll have to wait until monday when I'm back in the office. 🥳

[Elberet]

Test results: I'm getting an ExperimentalWarning: queueMicrotask() is experimental. and see only one Node process, but the behavior is unchanged.

In hindsight this is not much of a surprise. Event passing between threads is functionally very similar to managing worker processes; if there is a situation where the main node event pump can stall waiting for idle workers, the same can easily happen with threads.

[stefanpenner]

Ya, with node 11 at least we won’t orphan subprocesses.

If you have time to provide a reproduction or failing test, then I can be sure I am fixing the right bug.

I should have soon

[Elberet]

Here you go: https://github.com/Elberet/broccoli-babel-transpiler-test

[ef4]

I think I am seeing this bug. For me it only strikes on linux, I have yet to reproduce it on osx.

[stefanpenner]

I've also seen it on OSX....

[stefanpenner]

Taking a look now.

[stefanpenner]

So, I believe:

When the tree is processed, Babel will eventually be handed an options object with an unknownOption key and will (rightfully) raise a ReferenceError. So far, no surprises.

Is a red herring, the parallel nature of broccoli-babel-transpiler itself has never called terminate on the workerpool it spawns. I believe it relies on the process itself being exited.

If memory serves this is due to broccoli no longer calling "cleanup" in individual plugins, which then gives broccoli-persistent-filter no opportunity to shut its workers down. In addition, workers currently can be shared between pipelines, which increases the complexity somewhat.

Options:

• per broccoli pipeline pool of workers
• ref counting on the consumers of the worker pool

Next steps:

• make broccoli once again call cleanup on plugins, so that broccoli-babel-transpiler can terminate the workerpool
• explore the above two options

[stefanpenner]

broccoli-babel-transpiler works around this itself:

• https://github.com/babel/broccoli-babel-transpiler/blob/master/tests/utils/terminate-workers.js

• broccoli-babel-transpiler/tests/test.js

  Line 16 in a17ae29

  const terminateWorkerPool = require('./utils/terminate-workers');

• broccoli-babel-transpiler/tests/test.js

  Line 266 in a17ae29

  .then(terminateWorkerPool);

[loganpowell]

I tried using workerpool and got mixed results. If your code isn't locked to it, try threads.js.org

[rwjblue]

FWIW, this issue is still very frustrating. 😸

Ended up writing my own tiny broccoli babel compiler instead of reaching for the private APIs needed to ensure that the process exits. :(

[MelSumner]

@stefanpenner or @rwjblue is there any update on this issue?

[stefanpenner]

@MelSumner other then work-arounds, the root cause is in broccoli proper and most likely needs to be addressed via something such as -> broccolijs/broccoli#479