Skip to content

fix: proper SIGINT/SIGTERM handlers for graceful resource release (fixes #121) - #223

Open
dajiaohuang wants to merge 1 commit into
BabitMF:masterfrom
dajiaohuang:fix/signal-handler-issue-121
Open

dajiaohuang wants to merge 1 commit into
BabitMF:masterfrom
dajiaohuang:fix/signal-handler-issue-121

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Summary

This PR fixes issue #121 by implementing proper signal handling in the BMF C++ media framework engine, ensuring graceful resource release when the process receives termination signals.

Problems with the original implementation:

  1. Used unreliable std::signal() instead of POSIX sigaction()
  2. Called non-async-signal-safe functions directly from signal handlers (C++ iostreams, STL operations, mutexes)
  3. No protection against multiple rapid signals (double Ctrl+C could leave resources leaked)
  4. Race condition: signal handlers installed BEFORE graph added to global list, so early signals would crash
  5. Global graph list had no thread synchronization
  6. Graph destructor did not clean up the graph pointer from global list
  7. Only handled SIGINT and SIGTERM, missing SIGHUP
  8. g_ptr.clear() in close() would affect ALL graphs, not just the closing one

Changes made:

  • Use sigaction() instead of std::signal() for reliable, portable signal handling
  • Add SIGHUP support along with SIGINT and SIGTERM
  • Block signals during handler execution using sa_mask to prevent nested signal delivery
  • Double-signal protection: First signal triggers graceful shutdown; pressing Ctrl+C again forces immediate exit via _exit()
  • Async-signal-safe handler: Signal handler only uses safe operations (write(), atomic operations, _exit()); spawns a detached thread for complex cleanup
  • Thread-safe global list: Uses mutex to protect g_graphs vector from concurrent access
  • Fix registration order: Add graph to global list BEFORE installing signal handlers
  • Proper cleanup: Both close() and destructor remove only this graph from the list, not clear all graphs
  • Null safety: Check pointers before operating on them
  • User feedback: Clear messages printed to stderr indicating shutdown state

Files changed:

  • bmf/engine/c_engine/src/graph.cpp

Testing: Press Ctrl+C during graph execution - all nodes and resources should be properly closed. Press Ctrl+C again if stuck to force exit.

…ase (issue BabitMF#121)

- Replace unreliable std::signal() with POSIX sigaction() for robust signal handling
- Add support for SIGINT, SIGTERM, and SIGHUP signals
- Block signal delivery during handler execution to prevent race conditions
- Add double-signal protection: first signal triggers graceful shutdown, second signal forces immediate exit
- Use mutex-protected global graph list for thread safety
- Register graph instance in global list BEFORE signal handler installation to fix race condition
- Spawn detached cleanup thread to avoid async-signal-safety issues with complex C++ operations
- Properly remove graph pointer from global list on close() and destructor
- Use only async-signal-safe functions (write(), _exit()) directly in signal handler
- Added null-pointer safety check before graph operations
- Added thread header include for std::thread support
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