Race condition between os/exec.Cmd.Wait() and wago.Cmd.Kill()

[JonahBraun]

Summary

A known, harmless, race condition exists between these two functions:

  os/exec.(*Cmd).Wait()
  github.com/JonahBraun/wago.(*Cmd).Kill()

Details

We use os/exec.Cmd.Wait to wait for a process to exit and report on that. Before attempting a kill (in response to a FS event), in order to prevent unnecessary log errors, Wago's Cmd.Kill() checks exec.Cmd fields to see if the process is alive. Because Wait() and Kill() must occur in concurrent goroutines, a number of inherent race conditions exist.

These race conditions are harmless. The objective is to ensure a command is killed as quickly as possible in response to an event with the least number of extraneous warnings — and this works most of the time. In the rare worst case scenario, a process exits after the kill conditional

  if cmd == nil || cmd.Process == nil || cmd.ProcessState != nil {

but before the kill itself

  if err := cmd.Process.Kill(); err != nil {

In this case, an extraneous error is printed.

This could probably be resolved by rewriting os/exec to use channels instead fields for conveying process information. However, it should be noted that a race condition will always exist at the OS level. A process could exit in the middle of the Wago code to kill that command.

Please do test Wago with -race, just be aware of this known race condition.

[JonahBraun]

Runnables are responsible for checking kill chan at appropriate times.