Skip to content

fix: Add timeout to plugin stop requests, correctly propagate termination signals during graceful shutdown#131

Open
TheCrazyGM wants to merge 2 commits intohive-engine:qafrom
TheCrazyGM:shutdown-fix
Open

fix: Add timeout to plugin stop requests, correctly propagate termination signals during graceful shutdown#131
TheCrazyGM wants to merge 2 commits intohive-engine:qafrom
TheCrazyGM:shutdown-fix

Conversation

@TheCrazyGM
Copy link
Copy Markdown

Description

I run the node as a systemd service and I noticed it always timed out waiting for shutdown or restart. So I looked into it.

The current signal handling logic in app.js is flawed. It catches SIGINT/SIGTERM, performs cleanup, and then re-sends the signal to the process using process.kill(process.pid, signal). However, since the custom signal handler is still active, it catches the re-sent signal, checks the shuttingDown flag (which is now true), and simply returns, leaving the process running indefinitely.

Additionally, I found that during cleanup, some plugins (like P2P or Streamer) might not respond to the IPC message if they are in a bad state, causing unloadPlugin to hang indefinitely while awaiting the response.

Changes

  1. Refined Signal Propagation: In stopApp, explicitly remove the SIGINT/SIGTERM listeners before re-sending the signal. This ensures the default signal handler takes over and terminates the process correctly.
  2. Added Plugin Timeout: Added a 2-second timeout to the unloadPlugin function. If a plugin does not acknowledge the stop request within 2 seconds, the main process logs an error and force-kills the plugin, preventing the entire shutdown process from hanging.

Verification

  • Tested manually by running the node and sending SIGINT.
  • Verified that the process now exits immediately after "Saving config" with the correct exit signal, even if plugins are unresponsive.

@TheCrazyGM
Copy link
Copy Markdown
Author

After a few more tests, I believe 2 seconds is a bit harsh, should probably be increased for a bit more timeout.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant