Emulator shutdowns are not "clean" when using executors

[WhatsThatItsPat]

After using firebase emulators:start in the terminal and entering ctrl-c to end the process, I get lengthy output telling me about the shutdown process:

i  emulators: Received SIGINT (Ctrl-C) for the first time. Starting a clean shutdown.
i  emulators: Please wait for a clean shutdown or send the SIGINT (Ctrl-C) signal again to stop right now.

# ...and another dozen or more lines about shutting down the various emulators and exporting data

But after running nx serve functions or nx emulate functions, ending the process with ctrl-c doesn't show any of the output and I don't think everything is being shut down properly.

When I restart the emulators I get warnings about "multiple instances" or specific ports that are still in use:

emulators: It seems that you are running multiple instances of the emulator suite for project demo-local. This may result in unexpected behavior.

# or

firestore: Port 8080 is not open on localhost, could not start Firestore Emulator.

# or

ui: Emulator UI unable to start on port 4000, starting on 4001 instead.

Additionally, when I try to import & export emulator data, the data isn't exported to the correct place and extra firebase-export-{something} directories are created.

I've adjusted the command in the emulate executor to look like this:

firebase emulators:start --project=demo-local --import=./seed/demo-local --export-on-exit

When I run this directly in the terminal, everything shuts down fine and works as expected.

I'm on macOS, my firebase tools version is 9.16.6, and node is 16.8.0.

[jBernavaPrah]

Hi @WhatsThatItsPat,

What is your current workaround on this?

[WhatsThatItsPat]

I've just been running the firebase emulators:start... command directly while trying to remember to build things beforehand.

I've also added this to my package.json and use it if any ports are still running:

"scripts": {
  ...
  "kill-all": "npx kill-port 4000 5000 5001 8080 8085 9099 9199"
},

[LaysDragon]

Every time I terminate the emulator from nx serve firebase,there always a java process stuck at background and block me to restart the emulator. It seems the problem is @nrwl/workspace:run-commands unable to terminate emulator's multi process.

I end up using the concurrently to wrap up firebase emulate executor command. Because I accidentally founds out its can terminate emulator correctly,while I'm trying to use this cli tool to run my ng server and firebase emulator at the sametime.
Its seems silly to run only one command with this multiple-commands tool but its works :D
No,concurrently didn't really solve the problem. Its at least can terminated all emulator process,but still not gracefully shutdown the emulator.

    "emulate": {
      "executor": "@nrwl/workspace:run-commands",
      "options": {
        "command": "concurrently --raw \"firebase emulators:start --config firebase.json --only firestore,auth,storage,functions --import=emulator --export-on-exit=emulator\""
      }
    },

[LaysDragon]

I'm working under windows 10 with node v16.15.1.
Under nx 13,I have spend whole day on why emulator processes don't terminate gracefully. Have hard time to doing export-on-exit ,and have emulator process stuck in background.
The problem is come from nx itself. I not really familiar how process and signal working under different platform. But I manage to patch up nx 13 to make its able to gracefully shutdown the emulator under windows. D:

The nx cli have some weird design to exit itself immediately under SIGINT.
Including duplicated signal pass to executor process and child process,cause executor process exit immediately for unknown reason.
(The similar bug also happened to concurrently #283 cli tool too,why everyone doing this?)

Here is my nx patch up file if anyone need its. Its solve my problem and making everything working great, but I don't know if it will cause other problem D: I may need find time to to fire issue about this to nx repo.

nx+13.10.6.patch

diff --git a/node_modules/nx/src/tasks-runner/forked-process-task-runner.js b/node_modules/nx/src/tasks-runner/forked-process-task-runner.js
index 5cbc819..eb42bff 100644
--- a/node_modules/nx/src/tasks-runner/forked-process-task-runner.js
+++ b/node_modules/nx/src/tasks-runner/forked-process-task-runner.js
@@ -226,21 +226,21 @@ class ForkedProcessTaskRunner {
     setupOnProcessExitListener() {
         process.on('SIGINT', () => {
             this.processes.forEach((p) => {
-                p.kill('SIGTERM');
+                // p.kill('SIGTERM');
             });
             // we exit here because we don't need to write anything to cache.
-            process.exit();
+            // process.exit();
         });
         process.on('SIGTERM', () => {
             this.processes.forEach((p) => {
-                p.kill('SIGTERM');
+                // p.kill('SIGTERM');
             });
             // no exit here because we expect child processes to terminate which
             // will store results to the cache and will terminate this process
         });
         process.on('SIGHUP', () => {
             this.processes.forEach((p) => {
-                p.kill('SIGTERM');
+                // p.kill('SIGTERM');
             });
             // no exit here because we expect child processes to terminate which
             // will store results to the cache and will terminate this process

@nrwl+workspace+13.10.6.patch

diff --git a/node_modules/@nrwl/workspace/src/executors/run-commands/run-commands.impl.js b/node_modules/@nrwl/workspace/src/executors/run-commands/run-commands.impl.js
index 77fa9c9..7e7d380 100644
--- a/node_modules/@nrwl/workspace/src/executors/run-commands/run-commands.impl.js
+++ b/node_modules/@nrwl/workspace/src/executors/run-commands/run-commands.impl.js
@@ -35,6 +35,7 @@ const propKeys = [
     'outputPath',
 ];
 function default_1(options, context) {
+    ['exit','SIGTERM','SIGINT','SIGHUP'].forEach(signal=>process.once(signal, ()=>{}));
     return tslib_1.__awaiter(this, void 0, void 0, function* () {
         yield loadEnvVars(options.envFile);
         const normalized = normalizeOptions(options);
@@ -120,9 +121,11 @@ function createProcess(command, readyWhen, color, cwd) {
         /**
          * Ensure the child process is killed when the parent exits
          */
-        const processExitListener = () => childProcess.kill();
-        process.on('exit', processExitListener);
-        process.on('SIGTERM', processExitListener);
+        // const processExitListener = () => childProcess.kill();
+        // process.on('exit', processExitListener);
+        // process.on('SIGTERM', processExitListener);
+        // process.on('SIGINT', processExitListener);
+        // process.on('SIGHUP', processExitListener);
         childProcess.stdout.on('data', (data) => {
             process.stdout.write(data);
             if (readyWhen && data.toString().indexOf(readyWhen) > -1) {

[simondotm]

There's a fair bit of chat in the Nx repo about Nx task runner not terminating processes cleanly, and we have limited control over that.

The solution from @WhatsThatItsPat above works well so I've incorporated that into the latest plugin generator for the emulate target:

    "emulate": {
      "executor": "@nrwl/workspace:run-commands",
      "options": {
        "commands": [
          ...
          "kill-port --port 9099,5001,8080,9000,5000,8085,9199,9299,4000,4400,4500",
          "firebase emulators:start ..."
        ],
        "parallel": false
      }

It's not ideal because the cleanup only happens before starting the emulator, not when the task is ended, but its better than nothing.

Frankly, there's a lot of Nx edge cases that we can but hope get resolved one day.

[simondotm]

Looks like there might be a fix from Nx for this released soon: nrwl/nx#13885

[simondotm]

There's a further negative side effect of this Nx behaviour, which is that using --import and --export-on-exit for saving state of the firebase emulators doesn't work.

[simondotm]

FWIW I've decided to work around this by using an npm script for serving:

    "emulate": "kill-port --port 9099,5001,8080,9000,5000,8085,9199,9299,4000,4400,4500 && firebase emulators:start --project <project> --config firebase.json --import=apps/myapp/firebase/.emulators --export-on-exit",
    "serve": "npm run emulate & nx build myapp-firebase --watch",

This handles the sigint properly because we're calling firebase CLI outside of the Nx run-command wrapper.

(arguably the kill-port part is redundant if the emulator shuts down properly!)

[simondotm]

Another possible fix would be to have a custom 'serve' executor in the nx-firebase plugin that runs the necessary firebase command but also handles the sigint similarly to the patch above

[LaysDragon]

I felt they are trying so hard to dealing signal problem cross platform and still unable to deal its for all situation gracefully(signal propagating seems like a common problem with these cli tools(ex. concurrently from npm,melos from flutter)), as far as I known if the emulator didn't shutdown gracefully,the auto export won't work.

Making a specified executor for firebase emulator seems like a good idea,because we don't need to consider other situation from other program.

[simondotm]

Unfortunately even though that PR is now in Nx 16.1.1 it does not solve our issue - spawning the emulator from nx:run-commands still does not get the SIGINT sent to the child process for some reason. 😢

[Bullfrog1234]

Understand that it still an nx issue that is blocking this issue. To add a further temporary fix in your package I suggest that a export helper script is also added to the project. That way you can manual run the scipt before shutdown to export your data. This will mean that kill ports and export will at a minimum simulate a clean exit until they fix the problem.

[Bullfrog1234]

Bit of research on this problem

I think that firebase-tools is not handling a SIGTERM which is what is sent from NX on shutdown via either SUBINT. SUBTERM or SUBQUIT.

Here is the area of the code of NX that sends the signal to the sub-processes:
https://github.com/nrwl/nx/blob/e2ff519db2cd95168cee6f63c5338ce245d88d7a/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L501-L527

This means there is no clean shutdown, which includes exports and killing ports.

This is mentioned as a feature request in the firebase-tools repo to handle this feature firebase/firebase-tools#3578 They have added it to a roadmap but with no timeline.

It also meantioned in this issue on nx: nrwl/nx#18255

So this issue could be fixed by either package.

[simondotm]

I was giving this some more thought.
With the new v2 plugin project structure that is somewhat simplified, we have a firebase target that is used to invoke the firebase CLI using nx:run-command.

I'm thinking it might be possible to implement a custom executor in the plugin that calls the CLI directly and handles SIGTERM etc properly. Since run-command wont be involved, it might just work.... 🤔

[simondotm]

@Bullfrog1234 Thanks to your post above I think I've isolated the issue with Nx.

If we simply comment out process.exit() from line 487, the firebase cli gets its SIGINT and the serve command works fine.

https://github.com/nrwl/nx/blob/21d3c5e63c2e41b1f414d01194fbdb274a3185b2/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L480C1-L488C8

[simondotm]

BTW I made a start on a custom serve executor that spawns the watch & emulate commands internally, this also fixes the emulator cleanup problem, but I feel like its adding a lot of bloat to the plugin just to workaround an Nx issue.
I'll post up a draft PR for reference but would rather not pursue this approach if possible.

I'm wondering if a patch could be provided with the plugin somehow.

[simondotm]

I would welcome any feedback on the above PR. Will probably add it to next release.

[simondotm]

version 2.1.1 now supports clean shutdown of the emulator. 🙌
See also docs here.

[Bullfrog1234]

Thanks @simondotm sorry didn't get to review. Been off the grid for a while now. It looks amazing thanks for the amazing work.

[bojanbass]

I spent the whole day looking at my executor why it just kills the process immediately, without waiting for SIGINT handler. And then I realize that NX internally is doing process.exit() on SIGINT. I'm wondering why is this so?

[LaysDragon]

I spent the whole day looking at my executor why it just kills the process immediately, without waiting for SIGINT handler. And then I realize that NX internally is doing process.exit() on SIGINT. I'm wondering why is this so?

For some reason,seems like cli tools have common issue with process signal. Most of them just kill process or sending double SIGINT for no reason,I wondering why too.