veyron/tools/playground: Stop leaving orphan processes on failure.
We were using log.Fatal on errors, which calls os.Exit(1) without
calling deferred functions. This CL switches all instances of log.Fatal
to log.Panic, which allows deferred functions to run.
Change-Id: I5941e3042120401ce8abb2d8942082606baf7699
diff --git a/tools/playground/builder/builder.go b/tools/playground/builder/builder.go
index da168f2..c935e11 100644
--- a/tools/playground/builder/builder.go
+++ b/tools/playground/builder/builder.go
@@ -1,6 +1,17 @@
// Compiles and runs code for the Veyron playground.
// Code is passed via os.Stdin as a JSON encoded
// request struct.
+
+// NOTE(nlacasse): We use log.Panic() instead of log.Fatal() everywhere in this
+// file. We do this because log.Panic calls panic(), which allows any deferred
+// function to run. In particular, this will cause the mounttable and proxy
+// processes to be killed in the event of a compilation error. log.Fatal, on
+// the other hand, calls os.Exit(1), which does not call deferred functions,
+// and will leave proxy and mounttable processes running. This is not a big
+// deal for production environment, because the Docker instance gets cleaned up
+// after each run, but during development and testing these extra processes can
+// cause issues.
+
package main
import (
@@ -146,32 +157,32 @@
flag.Parse()
r, err := parseRequest(os.Stdin)
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
err = createIdentities(r.Identities)
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
mt, err := startMount(runTimeout)
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
defer mt.Kill()
proxy, err := startProxy()
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
defer proxy.Kill()
if err := writeFiles(r.Files); err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
if err := compileFiles(r.Files); err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
runFiles(r.Files)
@@ -395,13 +406,13 @@
stdout, err := cmd.StdoutPipe()
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
go streamEvents(fileName, "stdout", stdout)
stderr, err := cmd.StderrPipe()
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
go streamEvents(fileName, "stderr", stderr)
return cmd
@@ -413,7 +424,7 @@
writeEvent(fileName, scanner.Text(), stream)
}
if err := scanner.Err(); err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
}
@@ -427,7 +438,7 @@
jsonEvent, err := json.Marshal(e)
if err != nil {
- log.Fatal(err)
+ log.Panic(err)
}
// TODO(nlacasse): when we switch to streaming, we'll probably need to
diff --git a/tools/playground/builder/services.go b/tools/playground/builder/services.go
index b025339..9abfe21 100644
--- a/tools/playground/builder/services.go
+++ b/tools/playground/builder/services.go
@@ -1,4 +1,15 @@
// Functions to start services needed by the Veyron playground.
+
+// NOTE(nlacasse): We use log.Panic() instead of log.Fatal() everywhere in this
+// file. We do this because log.Panic calls panic(), which allows any deferred
+// function to run. In particular, this will cause the mounttable and proxy
+// processes to be killed in the event of a compilation error. log.Fatal, on
+// the other hand, calls os.Exit(1), which does not call deferred functions,
+// and will leave proxy and mounttable processes running. This is not a big
+// deal for production environment, because the Docker instance gets cleaned up
+// after each run, but during development and testing these extra processes can
+// cause issues.
+
package main
import (
@@ -55,7 +66,7 @@
}
endpoint := matches[1]
if endpoint == "" {
- log.Fatal("mounttable died")
+ log.Panic("mounttable died")
}
return cmd.Process, os.Setenv("NAMESPACE_ROOT", endpoint)
}
