Skip to content

Commit 48afa03

Browse files
authored
Merge pull request #6878 from zampani-docker/zampani/fix-plugin-force-exit-race
fix(cmd/docker): prevent race between force-exit goroutine and plugin wait
2 parents 12b8ca4 + 2bc4307 commit 48afa03

File tree

1 file changed

+29
-5
lines changed

1 file changed

+29
-5
lines changed

cmd/docker/docker.go

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,12 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command
344344
// notify the plugin via the PluginServer (or signal) as appropriate.
345345
const exitLimit = 2
346346

347+
// forceExitCh is closed by the signal goroutine just before it SIGKILLs
348+
// the plugin. The main goroutine checks this after plugincmd.Run() returns
349+
// and owns the final os.Exit(1) call, keeping exit-code ownership in one
350+
// place and avoiding a race between two concurrent os.Exit calls.
351+
forceExitCh := make(chan struct{})
352+
347353
tryTerminatePlugin := func(force bool) {
348354
// If stdin is a TTY, the kernel will forward
349355
// signals to the subprocess because the shared
@@ -368,12 +374,12 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command
368374

369375
// force the process to terminate if it hasn't already
370376
if force {
377+
// Close forceExitCh before Kill so the channel is guaranteed
378+
// to be closed by the time plugincmd.Run() returns: the plugin
379+
// can only exit after Kill() delivers SIGKILL, and Run() only
380+
// returns after the process is reaped.
381+
close(forceExitCh)
371382
_ = plugincmd.Process.Kill()
372-
_, _ = fmt.Fprint(dockerCli.Err(), "got 3 SIGTERM/SIGINTs, forcefully exiting\n")
373-
374-
// Restore terminal in case it was in raw mode.
375-
restoreTerminal(dockerCli)
376-
os.Exit(1)
377383
}
378384
}
379385

@@ -397,10 +403,28 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command
397403
force = true
398404
}
399405
tryTerminatePlugin(force)
406+
if force {
407+
// Plugin has been killed; return to prevent further
408+
// loop iterations from calling close(forceExitCh) again.
409+
return
410+
}
400411
}
401412
}()
402413

403414
if err := plugincmd.Run(); err != nil {
415+
select {
416+
case <-forceExitCh:
417+
// We force-killed the plugin after 3 signals. Print the message
418+
// and exit here so that exit-code ownership stays in the main
419+
// goroutine and we avoid a race with any concurrent os.Exit call.
420+
// Note: the deferred srv.Close() is already called by tryTerminatePlugin
421+
// before forceExitCh is closed, so skipping it here is safe.
422+
_, _ = fmt.Fprint(dockerCli.Err(), "got 3 SIGTERM/SIGINTs, forcefully exiting\n")
423+
restoreTerminal(dockerCli)
424+
os.Exit(1) //nolint:gocritic // exitAfterDefer: srv.Close() already called above
425+
default:
426+
}
427+
404428
statusCode := 1
405429
exitErr, ok := err.(*exec.ExitError)
406430
if !ok {

0 commit comments

Comments
 (0)