services/device/internal/impl: Use suidhelper kill functionality
Use killing via suidhelper to enforce app exit after a Stop() call.
While doing so, enable the deadline parameter to the Stop() call.
Also, use the suidhelper killing to kill off any newly-started app
that either fails to handshake properly or appears to lie about its pid
during the handshake.
In order to get this done without passing the suidhelper path around
everywhere, refactored the way that suidhelper is called to
centralize the knowledge of the suidhelper path and its flags in
helper_manager.go.
Change-Id: I84adcf874814bb040cfe8668da65e0193246c704
diff --git a/services/device/internal/impl/impl_test.go b/services/device/internal/impl/impl_test.go
index 0659978..ab399ce 100644
--- a/services/device/internal/impl/impl_test.go
+++ b/services/device/internal/impl/impl_test.go
@@ -65,6 +65,7 @@
deviceManagerCmd = "deviceManager"
deviceManagerV10Cmd = "deviceManagerV10" // deviceManager with a different major version number
appCmd = "app"
+ hangingAppCmd = "hangingApp"
installerCmd = "installer"
uninstallerCmd = "uninstaller"
@@ -236,6 +237,7 @@
type pingArgs struct {
Username, FlagValue, EnvValue string
+ Pid int
}
func ping(ctx *context.T) {
@@ -251,6 +253,7 @@
Username: savedArgs.Uname,
FlagValue: *flagValue,
EnvValue: os.Getenv(testEnvVarName),
+ Pid: os.Getpid(),
}
client := v23.GetClient(ctx)
if call, err := client.StartCall(ctx, "pingserver", "Ping", []interface{}{args}); err != nil {
@@ -303,6 +306,13 @@
return nil
}
+// Same as app, except that it does not exit properly after being stopped
+func hangingApp(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
+ err := app(stdin, stdout, stderr, env, args...)
+ time.Sleep(24 * time.Hour)
+ return err
+}
+
// TODO(rjkroege): generateDeviceManagerScript and generateSuidHelperScript have
// code similarity that might benefit from refactoring.
// generateDeviceManagerScript is very similar in behavior to generateScript in
@@ -592,17 +602,23 @@
// END HACK
}
-func verifyPingArgs(t *testing.T, pingCh <-chan pingArgs, username, flagValue, envValue string) {
+func receivePingArgs(t *testing.T, pingCh <-chan pingArgs) pingArgs {
var args pingArgs
select {
case args = <-pingCh:
case <-time.After(pingTimeout):
t.Fatalf(testutil.FormatLogLine(2, "failed to get ping"))
}
+ return args
+}
+
+func verifyPingArgs(t *testing.T, pingCh <-chan pingArgs, username, flagValue, envValue string) {
+ args := receivePingArgs(t, pingCh)
wantArgs := pingArgs{
Username: username,
FlagValue: flagValue,
EnvValue: envValue,
+ Pid: args.Pid, // We are not checking for a value of Pid
}
if !reflect.DeepEqual(args, wantArgs) {
t.Fatalf(testutil.FormatLogLine(2, "got ping args %q, expected %q", args, wantArgs))
@@ -854,6 +870,29 @@
// Starting new instances should no longer be allowed.
startAppExpectError(t, ctx, appID, impl.ErrInvalidOperation.ID)
+ // Make sure that Stop will actually kill an app that doesn't exit cleanly
+ // Do this by installing, starting, and stopping hangingApp, which sleeps (rather than
+ // exits) after being asked to Stop()
+ *envelope = envelopeFromShell(sh, nil, hangingAppCmd, "hanging ap", "hAppV1")
+ hAppID := installApp(t, ctx)
+ hInstanceID := startApp(t, ctx, hAppID)
+ hangingPid := receivePingArgs(t, pingCh).Pid
+ if err := syscall.Kill(hangingPid, 0); err != nil && err != syscall.EPERM {
+ t.Fatalf("Pid of hanging app (%v) is not live", hangingPid)
+ }
+ stopApp(t, ctx, hAppID, hInstanceID)
+ pidIsAlive := true
+ for i := 0; i < 10 && pidIsAlive; i++ {
+ if err := syscall.Kill(hangingPid, 0); err == nil || err == syscall.EPERM {
+ time.Sleep(time.Second) // pid is still alive
+ } else {
+ pidIsAlive = false
+ }
+ }
+ if pidIsAlive {
+ t.Fatalf("Pid of hanging app (%d) has not exited after Stop() call", hangingPid)
+ }
+
// Cleanly shut down the device manager.
defer verifyNoRunningProcesses(t)
syscall.Kill(dmh.Pid(), syscall.SIGINT)
