Code review comment for lp:~jamesodhunt/upstart/test-quiesce-cleanup

James Hunt (jamesodhunt) wrote :

Hi Steve,

The tests are simulating both System Shutdown and Session Shutdown.

For the Session Shutdown scenario, the Session Init will wait for the jobs, hence we expect all job processes (even those that block SIGTERM) to be dead by the time the Session Init exits since it will have sent such jobs SIGKILL.

However, in the System Shutdown scenario, when the Display Manager receives SIGTERM to denote system shutdown, it will send the same signal to all its clients, one of which is the Session Init. The Session Init will then send all job processes the job->class->kill_signal signal. However, since the Session Init should not hold up the system shutdown, although it attempts to wait for 5 seconds after sending SIGTERM before sending the final SIGKILL to each job process, it may already have been SIGKILL'd by the Display Manager: on an Ubuntu system, the DM waits 5 seconds between sending SIGTERM and SIGKILL... and the Session Init waits 5 seconds between the time it sent SIGTERM to each job process and the time it sends SIGKILL. The tests were attempting to simulate this exact behaviour but that does admittedly look "odd". I've now updated the branch to avoid such absolute mimicry but at the same time be more careful wrt checking which pids we expect to have stopped and which could still be running.

What this branch does highlight (and which the tests now comment on explicitly) is that any job that specifies "start on session-end" could still be running after the Session Init has exited but only in a System Shutdown context (since the "session-end" event is only emitted in the next iteration of the main loop - after Upstart has sent SIGTERM to all running job processes). As above, on an actual system those processes would get killed by the System Init.

« Back to merge proposal