canonical / pebble

Take control of your internal daemons!
GNU General Public License v3.0
137 stars 52 forks source link

fix: start process subreaper at top level to avoid shutdown hangs #380

Closed benhoyt closed 4 months ago

benhoyt commented 4 months ago

For example, if an exec command is running when you Ctrl-C the Pebble daemon, it hangs.

It's hanging because the ServiceManager is stopping the reaper early in the process, before TaskRunner has had a chance to abort the exec tasks (aborting an exec task sends SIGKILL to its pid via the tomb and command context).

Then when TaskRunner.Stop is called, it calls tomb.Kill and then tomb.Wait on each task (exec) tomb, and because the reaper's not running, the tomb.Wait hangs.

So the fix is to move the reaper.Start and reaper.Stop to the top level (inside the "run" command, which is also used for "enter"), instead of putting them in servstate.NewManager and ServiceManager.Stop.

After doing this, some of the tests also had to be modified to start and stop the reaper.

Fixes https://github.com/canonical/pebble/issues/163 and fixes #284