kvcache-ai/Mooncake

Fork 0

mirror of https://github.com/kvcache-ai/Mooncake.git synced 2026-11-05 01:17:55 +00:00

[GH-ISSUE #3271] [Bug]: TE Graceful shutdown may deadlock when SIGTERM or SIGINT is delivered to the watcher thread #834

New issue

Closed

opened 2026-10-29 21:20:01 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-29 21:20:01 +00:00

Owner

Originally created by @00fish0 on GitHub (Aug 4, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3271

Bug Report

Description

While stress-testing graceful_shutdown_test after fixing a separate child-readiness race, I found a lower-probability issue in the graceful shutdown implementation.

Mooncake uses a self-pipe pattern instead of cleaning up the TransferEngine directly inside the signal handler:

Signal-receiving thread              Watcher thread
          |                                |
          | signal handler                 | blocking read(pipe)
          | write signal to pipe --------->|
          | pause()                        | cleanupEngines()
                                           | _Exit(128 + signal)

This design is generally appropriate because complex cleanup is not async-signal-safe. Operations such as locking mutexes, releasing memory, or destroying threads may deadlock or corrupt state when executed directly inside a signal handler.

However, process-directed signals such as SIGTERM and SIGINT may be delivered to any thread that does not block them. The watcher thread currently does not block these signals.

If the signal is delivered to a normal thread, shutdown works as expected:

Normal thread receives the signal
    -> handler writes to the pipe
    -> watcher reads from the pipe
    -> watcher cleans up the engines
    -> process exits

If the signal is delivered to the watcher thread itself, the process can deadlock:

Watcher receives the signal
    -> watcher enters the signal handler
    -> handler writes the signal to the pipe
    -> handler waits in pause()
    -> no thread remains to read the pipe
    -> cleanup never runs
    -> process never exits

In other words, the only pipe reader becomes blocked inside the signal handler after writing a message to itself.

Steps to reproduce

Running multiple test processes concurrently increases the reproduction probability:

seq 1 400 | xargs -P 8 -I ID \
  build/mooncake-transfer-engine/tests/graceful_shutdown_test \
  --gtest_brief=1 \
  --gtest_shuffle \
  --gtest_random_seed=ID

The child did not exit within the five-second timeout and had to be terminated with SIGKILL. Example failure:

[ RUN      ] GracefulShutdownTest.SigintTriggersCleanExit
graceful_shutdown_test.cpp: Failure
child did not exit before timeout

Value of: WIFEXITED(status)
  Actual: false
Expected: true

[  FAILED  ] GracefulShutdownTest.SigintTriggersCleanExit

Expected behavior

The watcher thread should remain available to read the self-pipe and perform cleanup regardless of which process-directed signal is received.

After receiving SIGTERM or SIGINT, the process should clean up registered engines and exit with 128 + signal, for example:

143 for SIGTERM
130 for SIGINT

Actual behavior

If the signal is delivered to the watcher thread, the watcher enters the signal handler, writes to its own pipe, and waits indefinitely. No thread remains to consume the pipe notification, so cleanup and process exit never occur.

Possible fix

Ensure that the watcher thread cannot receive SIGTERM or SIGINT.
One possible approach is:

Temporarily block SIGTERM and SIGINT in the thread creating the watcher.
Create the watcher so that it inherits the blocked signal mask.
Restore the creating thread’s original signal mask.
Keep the signals blocked in the watcher for its entire lifetime.

For example, this can be implemented using pthread_sigmask() around watcher thread creation.

All failure paths should restore the caller’s original signal mask and close any newly created pipe file descriptors.

Impact

This can cause a process to hang indefinitely during graceful shutdown. In containerized or orchestrated environments, the process may eventually be force-killed after its termination grace period, preventing normal Transfer Engine cleanup.

The issue is timing-dependent and may be difficult to reproduce in a single test run, but it was observed during repeated stress testing.

Before submitting...

Ensure you searched for relevant issues and read the [documentation]

Originally created by @00fish0 on GitHub (Aug 4, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3271 ### Bug Report ## Description While stress-testing graceful_shutdown_test after fixing a separate child-readiness race, I found a lower-probability issue in the graceful shutdown implementation. Mooncake uses a self-pipe pattern instead of cleaning up the TransferEngine directly inside the signal handler: ``` Signal-receiving thread Watcher thread | | | signal handler | blocking read(pipe) | write signal to pipe --------->| | pause() | cleanupEngines() | _Exit(128 + signal) ``` This design is generally appropriate because complex cleanup is not async-signal-safe. Operations such as locking mutexes, releasing memory, or destroying threads may deadlock or corrupt state when executed directly inside a signal handler. However, process-directed signals such as SIGTERM and SIGINT may be delivered to any thread that does not block them. The watcher thread currently does not block these signals. If the signal is delivered to a normal thread, shutdown works as expected: ``` Normal thread receives the signal -> handler writes to the pipe -> watcher reads from the pipe -> watcher cleans up the engines -> process exits ``` **If the signal is delivered to the watcher thread itself, the process can deadlock:** ``` Watcher receives the signal -> watcher enters the signal handler -> handler writes the signal to the pipe -> handler waits in pause() -> no thread remains to read the pipe -> cleanup never runs -> process never exits ``` In other words, the only pipe reader becomes blocked inside the signal handler after writing a message to itself. ## Steps to reproduce Running multiple test processes concurrently increases the reproduction probability: ``` seq 1 400 | xargs -P 8 -I ID \ build/mooncake-transfer-engine/tests/graceful_shutdown_test \ --gtest_brief=1 \ --gtest_shuffle \ --gtest_random_seed=ID ``` The child did not exit within the five-second timeout and had to be terminated with SIGKILL. Example failure: ``` [ RUN ] GracefulShutdownTest.SigintTriggersCleanExit graceful_shutdown_test.cpp: Failure child did not exit before timeout Value of: WIFEXITED(status) Actual: false Expected: true [ FAILED ] GracefulShutdownTest.SigintTriggersCleanExit ``` ## Expected behavior The watcher thread should remain available to read the self-pipe and perform cleanup regardless of which process-directed signal is received. After receiving SIGTERM or SIGINT, the process should clean up registered engines and exit with 128 + signal, for example: - 143 for SIGTERM - 130 for SIGINT ## Actual behavior If the signal is delivered to the watcher thread, the watcher enters the signal handler, writes to its own pipe, and waits indefinitely. No thread remains to consume the pipe notification, so cleanup and process exit never occur. ## Possible fix Ensure that the watcher thread cannot receive SIGTERM or SIGINT. One possible approach is: 1. Temporarily block SIGTERM and SIGINT in the thread creating the watcher. 2. Create the watcher so that it inherits the blocked signal mask. 3. Restore the creating thread’s original signal mask. 4. Keep the signals blocked in the watcher for its entire lifetime. For example, this can be implemented using pthread_sigmask() around watcher thread creation. All failure paths should restore the caller’s original signal mask and close any newly created pipe file descriptors. ## Impact This can cause a process to hang indefinitely during graceful shutdown. In containerized or orchestrated environments, the process may eventually be force-killed after its termination grace period, preventing normal Transfer Engine cleanup. The issue is timing-dependent and may be difficult to reproduce in a single test run, but it was observed during repeated stress testing. ### Before submitting... - [ ] Ensure you searched for relevant issues and read the [documentation]

bsakaguchi

2026-10-29 21:20:01 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-29 21:20:01 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Aug 4, 2026):

Thanks for opening this issue, @00fish0!

Field	Value
Issue	#3271
GitHub user ID	`50252876`
Reporter	@00fish0

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Aug 4, 2026): Thanks for opening this issue, @00fish0! | Field | Value | |-------|-------| | **Issue** | #3271 | | **GitHub user ID** | `50252876` | | **Reporter** | @00fish0 | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:20:01 +00:00

Author

Owner

@he-yufeng commented on GitHub (Aug 4, 2026):

Verified the mechanism against the code; the report is exact.

installGracefulShutdownHandlers() spawns signalWatcher detached, and the watcher then sits in read(g_signal_pipe[0], ...). Nothing blocks SIGTERM/SIGINT for that thread, so a process-directed signal can be delivered to the watcher itself. The handler then runs on the watcher thread: writes one byte into the pipe, and parks in for (;;) pause(). The only pipe reader is now suspended inside the handler, the byte is never consumed, cleanupEngines() never runs, and the process hangs until something SIGKILLs it. The fork-path guard (g_handlers_pid != getpid()) covers children, not this case.

Your masked-creation approach is the right fix. Concretely in startSignalWatcherLocked(): pthread_sigmask(SIG_BLOCK, {SIGTERM, SIGINT}, &saved) before std::thread(signalWatcher).detach(), restore the saved mask right after, restore on every failure path too. The watcher then inherits the blocked mask for its whole lifetime, the signal always lands on some other thread, and the pipe protocol works as designed. If pthread_sigmask itself fails we keep today's behavior rather than refusing to install, since the window is narrow and the current code has the same exposure.

One extra detail worth stating in the fix: restoring the creator's mask (not just unblocking) matters, because the caller may already have had SIGTERM blocked for its own reasons, and installGracefulShutdownHandlers() runs per engine under g_install_mutex, so the block/restore pair has to be balanced on every call.

@00fish0 you filed this with a full analysis, so first dibs: are you planning to send the PR yourself? If not I have the change ready and can open it.

@he-yufeng commented on GitHub (Aug 4, 2026): Verified the mechanism against the code; the report is exact. `installGracefulShutdownHandlers()` spawns `signalWatcher` detached, and the watcher then sits in `read(g_signal_pipe[0], ...)`. Nothing blocks SIGTERM/SIGINT for that thread, so a process-directed signal can be delivered to the watcher itself. The handler then runs on the watcher thread: writes one byte into the pipe, and parks in `for (;;) pause()`. The only pipe reader is now suspended inside the handler, the byte is never consumed, `cleanupEngines()` never runs, and the process hangs until something SIGKILLs it. The fork-path guard (`g_handlers_pid != getpid()`) covers children, not this case. Your masked-creation approach is the right fix. Concretely in `startSignalWatcherLocked()`: `pthread_sigmask(SIG_BLOCK, {SIGTERM, SIGINT}, &saved)` before `std::thread(signalWatcher).detach()`, restore the saved mask right after, restore on every failure path too. The watcher then inherits the blocked mask for its whole lifetime, the signal always lands on some other thread, and the pipe protocol works as designed. If `pthread_sigmask` itself fails we keep today's behavior rather than refusing to install, since the window is narrow and the current code has the same exposure. One extra detail worth stating in the fix: restoring the creator's mask (not just unblocking) matters, because the caller may already have had SIGTERM blocked for its own reasons, and `installGracefulShutdownHandlers()` runs per engine under `g_install_mutex`, so the block/restore pair has to be balanced on every call. @00fish0 you filed this with a full analysis, so first dibs: are you planning to send the PR yourself? If not I have the change ready and can open it.

bsakaguchi commented

2026-10-29 21:20:02 +00:00

Author

Owner

@he-yufeng commented on GitHub (Aug 5, 2026):

Follow-up: with no response for a day, I went ahead and opened the fix as #3278. It does exactly the masked-creation sequence described above (block SIGTERM/SIGINT before creating the watcher thread, restore the creator's mask on all paths, keep current behavior if pthread_sigmask itself fails). alogfans has already had a first look.

@he-yufeng commented on GitHub (Aug 5, 2026): Follow-up: with no response for a day, I went ahead and opened the fix as #3278. It does exactly the masked-creation sequence described above (block SIGTERM/SIGINT before creating the watcher thread, restore the creator's mask on all paths, keep current behavior if pthread_sigmask itself fails). alogfans has already had a first look.

bsakaguchi referenced this issue

2026-10-29 21:31:10 +00:00

[PR #902] [MERGED] [Store]: Add option to use jemalloc in mooncake store master #1893