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

SIGINT is sent twice when pressing Ctrl-C, causing dirty shutdown #283

Open
alimony opened this issue Jun 24, 2021 · 19 comments
Open

SIGINT is sent twice when pressing Ctrl-C, causing dirty shutdown #283

alimony opened this issue Jun 24, 2021 · 19 comments

Comments

@alimony
Copy link

alimony commented Jun 24, 2021

I have a package.json with the following scripts:

…
"startemulators": "firebase emulators:start --import seed",
"listen": "onchange 'source_files/*.js' -- touch functions/index.js",
"emulate": "concurrently \"npm run startemulators\" \"npm run listen\""
…

Running the emulate script works as expected, but pressing Ctrl-C to shut the processes down gives me this:

[0] i  emulators: Received SIGINT (Ctrl-C) for the first time. Starting a clean shutdown.
[0] i  emulators: Please wait for a clean shutdown or send the SIGINT (Ctrl-C) signal again to stop right now.
[0] i  emulators: Shutting down emulators.
[0] i  ui: Stopping Emulator UI
[0] ⚠  Emulator UI has exited upon receiving signal: SIGINT
[0] i  functions: Stopping Functions Emulator
[0] i  hosting: Stopping Hosting Emulator
[0] i  database: Stopping Database Emulator
[1] npm run listen exited with code 0
[0]  
[0] ⚠  emulators: Received SIGINT (Ctrl-C) 2 times. You have forced the Emulator Suite to exit without waiting for 3 subprocesses to finish.

So for some reason, a second SIGINT is sent to the firebase command specified in the first script. How come?

@alimony
Copy link
Author

alimony commented Jul 5, 2021

Any ideas on this?

@akauppi
Copy link

akauppi commented Jul 5, 2021

@alimony I have seen this, but wasn't considering concurrently would be causing the double Ctrl-C's.

It may well be. In that case, debugging it means having a project with Firebase CLI and concurrently both in the mix. Maybe we should create a minimal project that showcases this?


I can reproduce it:

⚠  emulators: Received SIGINT (Ctrl-C) 2 times. You have forced the Emulator Suite to exit without waiting for 1 subprocess to finish. These processes may still be running on your machine: 

┌────────────────────┬──────────────┬─────┐
│ Emulator           │ Host:Port    │ PID │
├────────────────────┼──────────────┼─────┤
│ Firestore Emulator │ 0.0.0.0:6767 │ 32  │
└────────────────────┴──────────────┴─────┘

macOS 11.4, firebase-tools 9.12.1, concurrently 6.2.0

@alimony
Copy link
Author

alimony commented Jul 5, 2021

I'll create a minimal test case.

@akauppi
Copy link

akauppi commented Jul 5, 2021

I can provide a reproducible, negative test case.

$ git clone https://github.com/akauppi/firebase-jest-testing.git
$ cd firebase-jest-testing
$ git checkout next     # ADDED THIS LATER!
$ npm install
$ npm run //start:pipeless
...
<once services are up, press Ctrl-C>

Tried ~6 times, not a single time there is Received SIGINT (Ctrl-C) 2 times.


I'll try to compare with my other project, where I do see these, and try to spot the difference.

The weird "//start:pipeless" is because if I pipe Firebase Emulator output, another concurrently issue is raised, and it disturbs in checking this one. :)

macOS 11.4

@akauppi
Copy link

akauppi commented Jul 5, 2021

The positive case comes here.

If I launch: concurrently -> Docker -> Firebase CLI (which I do in a web app project, all the time), the double Ctrl-C occurs.

  • It's always connected to using Docker
  • diving into the container and launching it manually from there (no concurrently involved) doesn't recreate the error
Detailed log
> concurrently --kill-others "$(docker-run-cmd) firebase emulators:start --project=demo-2"

Launching Docker... 🐳



[0] ⚠  emulators: You are not currently authenticated so some features may not work correctly. Please run firebase login to authenticate the CLI.
[0] i  emulators: Starting emulators: auth, functions, firestore
[0] i  emulators: Detected demo project ID "demo-2", emulated services will use a demo configuration and attempts to access non-emulated services for this project will fail.
[0] ⚠  Your requested "node" version "14 || ^16" doesn't match your global version "16"
[0] ⚠  functions: You are not signed in to the Firebase CLI. If you have authorized this machine using gcloud application-default credentials those may be discovered and used to access production services.
[0] i  firestore: Firestore Emulator logging to firestore-debug.log
[0] i  ui: Emulator UI logging to ui-debug.log
[0] i  functions: Watching "/work/functions" for Cloud Functions...
[0] ✔  functions[us-central1-cloudLoggingProxy_v0]: http function initialized (http://0.0.0.0:5002/demo-2/us-central1/cloudLoggingProxy_v0).
[0] ✔  functions[us-central1-userInfoShadow_2]: firestore function initialized.
[0] 
[0] ┌─────────────────────────────────────────────────────────────┐
[0] │ ✔  All emulators ready! It is now safe to connect your app. │
[0] │ i  View Emulator UI at http://0.0.0.0:4000                  │
[0] └─────────────────────────────────────────────────────────────┘
[0] 
[0] ┌────────────────┬──────────────┬───────────────────────────────┐
[0] │ Emulator       │ Host:Port    │ View in Emulator UI           │
[0] ├────────────────┼──────────────┼───────────────────────────────┤
[0] │ Authentication │ 0.0.0.0:9100 │ http://0.0.0.0:4000/auth      │
[0] ├────────────────┼──────────────┼───────────────────────────────┤
[0] │ Functions      │ 0.0.0.0:5002 │ http://0.0.0.0:4000/functions │
[0] ├────────────────┼──────────────┼───────────────────────────────┤
[0] │ Firestore      │ 0.0.0.0:6767 │ http://0.0.0.0:4000/firestore │
[0] └────────────────┴──────────────┴───────────────────────────────┘
[0]   Emulator Hub running at localhost:4400
[0]   Other reserved ports: 4500
[0] 
[0] Issues? Report them at https://github.com/firebase/firebase-tools/issues and attach the *-debug.log files.
[0]  
^C[0]  
[0] i  emulators: Received SIGINT (Ctrl-C) for the first time. Starting a clean shutdown.
[0] i  emulators: Please wait for a clean shutdown or send the SIGINT (Ctrl-C) signal again to stop right now.
[0] i  emulators: Shutting down emulators.
[0] i  ui: Stopping Emulator UI
[0] ⚠  Emulator UI has exited upon receiving signal: SIGINT
[0] i  functions: Stopping Functions Emulator
[0] i  firestore: Stopping Firestore Emulator
[0]  
[0] ⚠  emulators: Received SIGINT (Ctrl-C) 2 times. You have forced the Emulator Suite to exit without waiting for 1 subprocess to finish. These processes may still be running on your machine: 
[0] 
[0] ┌────────────────────┬──────────────┬─────┐
[0] │ Emulator           │ Host:Port    │ PID │
[0] ├────────────────────┼──────────────┼─────┤
[0] │ Firestore Emulator │ 0.0.0.0:6767 │ 30  │
[0] └────────────────────┴──────────────┴─────┘
[0] 
[0] To force them to exit run:
[0] 
[0] kill 30
[0] 
[0] docker run --rm --sig-proxy=true -v /Users/asko/Git/GroundLevel-firebase-es/packages/backend:/work -w /work -p 6767:6767 -p 5002:5002 -p 9100:9100 -p 4000:4000 firebase-ci-builder:9.12.1-node16-npm7 firebase emulators:start --project=demo-2 exited with code 0

Note: This is just my 2c. I am not really troubled by this error message, since if running things under Docker, things get cleaned up, anyhow.

@alimony
Copy link
Author

alimony commented Jul 5, 2021

I have not been able to create a small test case that consistently shows this error, but I have discovered that skipping the pubsub emulator makes the error go away. For example, using this:

firebase emulators:start --only firestore,database,functions,hosting,pubsub

will send SIGINT twice when quitting concurrently, while this:

firebase emulators:start --only firestore,database,functions,hosting

will not.

@ghivert
Copy link

ghivert commented Jul 12, 2021

Hi,

I have the same problem and investigated a little bit.
By commenting out that line, I ended up with something working correctly, no double SIGINT, everything is cool.

It looks like spawn-command does not spawn the subprocess in detached mode, and it seems the defaults in concurrently are not setting detached options neither. Because if not, it's perfectly normal to have the SIGINT sent twice, because it's received by the subprocess, and then resent with command.kill(signal) a second time.

@idudinov
Copy link

hi there!
I'm experiencing this issue too if I try to run firebase directly as concurrently command.

However, if my firebase command is wrapped with yarn script command in package.json, the issue is not reproduced. Looks like yarn already does some protection of double sent signals.

I can't use full command in scripts since I construct firebase command args on the fly.
So I did an alias like that:

// package.json

"scripts": {
  "firebase:emulators": "firebase emulators:start",
}

and then call the command via concurrently like that:

yarn firebase:emulators --only ... <here are my dynamic args>

for me this is a legit workaround, hope it helps someone else

@samtstern
Copy link

Running into this problem myself, here's a bit from my package.json:

  "scripts": {
    "build": "tsc",
    "build:watch": "tsc -w",
    "format": "prettier --write src/**/*.ts",
    "emulate": "firebase emulators:start --import=./emulator-data --export-on-exit",
    "serve": "concurrently 'npm run build:watch' 'npm run emulate'"
  },

If I run npm run serve and then press Ctrl+C the Firebase emulators see double SIGINT signals and shutdown without exporting data.

@gustavohenke
Copy link
Member

Hey all. I've briefly looked at this today, it seems that firebase-tools@9 causes the issue, but firebase-tools@10 doesn't.

@akauppi it seems you did a fair amount of research in firebase/firebase-tools#3578.
Is there much that should be done here?

I see people mentioned the removal of things like passing SIGINT down to the command or spawning subprocesses in detached mode, but I'm afraid this would worsen issues around hanging processes.

@akauppi
Copy link

akauppi commented Jan 2, 2022

@gustavohenke I moved from concurrently to Docker Containers last year, and that removed the dirty shutdown issue for me.

@jakeleventhal
Copy link

jakeleventhal commented Feb 26, 2022

Same issue with firebase export

@jakeleventhal
Copy link

A solution i came up with is like this:

{
    "emulator-start": "./node_modules/.bin/firebase emulators:start --import=emulatorData/localEmulatorData --export-on-exit=emulatorData/localEmulatorData",
    "start": "npm run emulator-start & npm run start-servers",
    "start-servers": "concurrently -c gray.dim --kill-others \"npm run api\" \"npm run scheduler\""
}

@jasonkylefrank
Copy link

@jakeleventhal So you just replaced concurrently with the & approach for the npm script which combines the emulators with another command?

My understanding is that the & approach will not work on Windows machines. Is that still true?(I'm on Mac and don't have a Windows machine handy to test it with).

@jakeleventhal
Copy link

@jasonkylefrank kind of - i am running the emulators outside of concurrently but my servers are still running via concurrently

as for windows - i don't use windows so idk lol

@hixus
Copy link

hixus commented May 3, 2022

I noticed this today as now I can't exit the shell with pressing ctrl+c again (on macbook pro).

running "concurrently \"yarn:firebase:emulators\"" and pressing ctrl+c gives exit code 1. Running the same without concurrently gives exit code 0.

Adding docker to mix with "concurrently \"yarn:firebase:emulators\" \"docker compose up\" and pressing ctrl+c I get

^C[1] Gracefully stopping... (press Ctrl+C again to force)
[firebase:emulators]
[firebase:emulators] i  emulators: Received SIGINT (Ctrl-C) for the first time. Starting a clean shutdown.
[firebase:emulators] i  emulators: Please wait for a clean shutdown or send the SIGINT (Ctrl-C) signal again to stop right now.
[firebase:emulators] i  emulators: Shutting down emulators.
[firebase:emulators] i  ui: Stopping Emulator UI
[firebase:emulators] ⚠  Emulator UI has exited upon receiving signal: SIGINT
[firebase:emulators] i  auth: Stopping Authentication Emulator
[firebase:emulators] i  hub: Stopping emulator hub
[1] Container gql-1  Stopping
[firebase:emulators] i  logging: Stopping Logging Emulator
[1] time="2022-05-03T08:09:27+03:00" level=error msg="got 3 SIGTERM/SIGINTs, forcing shutdown"
[1] Container db-1  Killing
[1] Container gql-1  Killing
[firebase:emulators] yarn run firebase:emulators exited with code 1
[1] Container db-1  Killed
[1] Container gql-1  Killed
[1] Container gql-1  Stopped
[1] Container db-1  Stopping
[1] Container db-1  Stopped

concurrently process seems to be stuck itself so I cant exit the shell anymore

@LaysDragon
Copy link

LaysDragon commented Jul 16, 2022

@gustavohenke
I see people mentioned the removal of things like passing SIGINT down to the command or spawning subprocesses in detached mode, but I'm afraid this would worsen issues around hanging processes.

  • windows 10
  • firebase-tool 11.2.2
  • concurrently@7.2.2

I'm also have this problem that double SIGINT cause emulator unable to export on exit gracefully.
I remove and patched the line that @ghivert mentioned,and its really solve the problem on windows. Double SIGINT seems really not a right way to exit all process, maybe consider use the timeout timer to deal with hanging process in any case?

@Zehir

This comment was marked as off-topic.

@jahudka
Copy link

jahudka commented Aug 12, 2023

I think this may be due to Concurrently's use of tree-kill to terminate individual processes. For example, if you send a SIGINT signal to a Docker Compose, it will handle terminating its subprocesses on its own; there's no need for Concurrently to know or care that the process has subprocesses (although I'm sure that's not always the case). But I think having the option to override this behaviour with simply sending the signal to each of the original processes might solve this issue.

I wanted to verify this by writing a minimal wrapper for the Concurrently API and passing (pid, signal) => process.kill(pid, signal) as the kill option; but it doesn't quite seem to work as it should - there's no output at all (even when I pass outputStream: process.stdout in the options), and the script seems to terminate immediately when I hit Ctrl+C, as opposed to waiting for the child processes to exit, like the CLI does; so I don't really know how to verify this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests