DefaultCommand signal behavior improvements for plugins (#570)

## Type of Change
- [x] Bug fix
- [ ] New feature  
- [ ] Breaking change
- [ ] Documentation update

## Description
Correct signal semantics for plugins: Container binary currently execs
into plugin binaries. If the parent CLI keeps SIGINT/SIGTERM handlers
installed, it can intercept/alter signal behavior intended for the
plugin (e.g., preventing graceful shutdown in foreground workflows).

## Motivation and Context
During the development of a plugin (docker compose compatibility
plugin), I encountered a major issues where CTRL-C (SIGTERM) was not
being sent to my plugin. CLI plugins, especially those that have long
running tasks need a way to handle signals from the OS. Current, we exec
into plugin binaries. If the parent CLI keeps SIGINT/SIGTERM handlers
installed, it can intercept/alter signal behavior intended for the
plugin (e.g., preventing graceful shutdown in foreground workflows).

### What we changed:
- Signals handed back to plugins: 
- DefaultCommand resets SIGINT/SIGTERM to defaults immediately before
exec’ing the plugin.
- Rationale: since exec replaces the process image, signals should be
delivered to (and handled by) the plugin without parent interference.
  - Non‑plugin commands remain unaffected by this change.
  - Compatibility: No change to plugin ABI or exec flow.

### Alternatives considered:
- Supervising child instead of exec: central forwarding of signals from
parent to plugin. Rejected for now to avoid changing process tree/stdio
semantics; resetting to defaults before exec preserves current model
while fixing signal interference.

## Testing
- [X] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
This commit is contained in:
mazdak
2025-09-05 20:49:54 -07:00
committed by GitHub
parent 7bd9e5ac82
commit bd2c228b57
+11
View File
@@ -17,6 +17,7 @@
import ArgumentParser
import ContainerClient
import ContainerPlugin
import Darwin
struct DefaultCommand: AsyncParsableCommand {
static let configuration = CommandConfiguration(
@@ -48,7 +49,17 @@ struct DefaultCommand: AsyncParsableCommand {
guard let plugin = pluginLoader?.findPlugin(name: command), plugin.config.isCLI else {
throw ValidationError("failed to find plugin named container-\(command)")
}
// Before execing into the plugin, restore default SIGINT/SIGTERM so the plugin can manage signals.
Self.resetSignalsForPluginExec()
// Exec performs execvp (with no fork).
try plugin.exec(args: remaining)
}
}
extension DefaultCommand {
// Exposed for tests to verify signal reset semantics.
static func resetSignalsForPluginExec() {
signal(SIGINT, SIG_DFL)
signal(SIGTERM, SIG_DFL)
}
}