Merge "veyron/runtimes/google/ipc: Introduce a NoRetry opt to make ipc tests run much faster."
diff --git a/lib/exec/child.go b/lib/exec/child.go
index 09b53fe..688a74b 100644
--- a/lib/exec/child.go
+++ b/lib/exec/child.go
@@ -26,6 +26,15 @@
// veyron framework to notify the parent that the child process has
// successfully started.
statusPipe *os.File
+
+ // statusMu protexts sentStatus and statusErr and prevents us from trying to
+ // send multiple status updates to the parent.
+ statusMu sync.Mutex
+ // sentStatus records the status that was already sent to the parent.
+ sentStatus string
+ // statusErr records the error we received, if any, when sentStatus
+ // was sent.
+ statusErr error
}
var (
@@ -57,35 +66,43 @@
return childHandle, childHandleErr
}
-func (c *ChildHandle) writeStatus(status string) error {
- toWrite := make([]byte, 0, len(status))
- var buf [utf8.UTFMax]byte
- // This replaces any invalid utf-8 bytes in the status string with the
- // Unicode replacement character. This ensures that we only send valid
- // utf-8 (followed by the eofChar).
- for _, r := range status {
- n := utf8.EncodeRune(buf[:], r)
- toWrite = append(toWrite, buf[:n]...)
+func (c *ChildHandle) writeStatus(status, detail string) error {
+ c.statusMu.Lock()
+ defer c.statusMu.Unlock()
+
+ if c.sentStatus == "" {
+ c.sentStatus = status
+ status = status + detail
+ toWrite := make([]byte, 0, len(status))
+ var buf [utf8.UTFMax]byte
+ // This replaces any invalid utf-8 bytes in the status string with the
+ // Unicode replacement character. This ensures that we only send valid
+ // utf-8 (followed by the eofChar).
+ for _, r := range status {
+ n := utf8.EncodeRune(buf[:], r)
+ toWrite = append(toWrite, buf[:n]...)
+ }
+ toWrite = append(toWrite, eofChar)
+ _, c.statusErr = c.statusPipe.Write(toWrite)
+ c.statusPipe.Close()
+ } else if c.sentStatus != status {
+ return errors.New("A different status: " + c.sentStatus + " has already been sent.")
}
- toWrite = append(toWrite, eofChar)
- _, err := c.statusPipe.Write(toWrite)
- c.statusPipe.Close()
- return err
+ return c.statusErr
}
-// TODO(caprita): There's nothing preventing SetReady and SetFailed from being
-// called multiple times (e.g. from different instances of the runtime
-// intializing themselves). This results in errors for all but the first
-// invocation. Should we instead run these with sync.Once?
-
// SetReady writes a 'ready' status to its parent.
+// Only one of SetReady or SetFailed can be called, attempting to send
+// both will fail. In addition the status is only sent once to the parent
+// subsequent calls will return immediately with the same error that was
+// returned on the first call (possibly nil).
func (c *ChildHandle) SetReady() error {
- return c.writeStatus(readyStatus + strconv.Itoa(os.Getpid()))
+ return c.writeStatus(readyStatus, strconv.Itoa(os.Getpid()))
}
// SetFailed writes a 'failed' status to its parent.
func (c *ChildHandle) SetFailed(oerr error) error {
- return c.writeStatus(failedStatus + oerr.Error())
+ return c.writeStatus(failedStatus, oerr.Error())
}
// NewExtraFile creates a new file handle for the i-th file descriptor after
diff --git a/lib/exec/exec_test.go b/lib/exec/exec_test.go
index f2ca9db..44f92a5 100644
--- a/lib/exec/exec_test.go
+++ b/lib/exec/exec_test.go
@@ -536,17 +536,28 @@
log.Fatal(err)
}
verifyNoExecVariable()
- if err := ch.SetFailed(fmt.Errorf("%s", strings.Join(args, " "))); err != nil {
-
+ err = ch.SetFailed(fmt.Errorf("%s", strings.Join(args, " ")))
+ if err != nil {
fmt.Fprintf(os.Stderr, "%s\n", err)
}
+ // It's fine to call SetFailed multiple times.
+ if err2 := ch.SetFailed(fmt.Errorf("dummy")); err != err2 {
+ fmt.Fprintf(os.Stderr, "Received new error got: %v, want %v\n", err2, err)
+ }
case "testReady":
ch, err := vexec.GetChildHandle()
if err != nil {
log.Fatal(err)
}
verifyNoExecVariable()
- ch.SetReady()
+ err = ch.SetReady()
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "%s\n", err)
+ }
+ // It's fine to call SetReady multiple times.
+ if err2 := ch.SetReady(); err != err2 {
+ fmt.Fprintf(os.Stderr, "Received new error got: %v, want %v\n", err2, err)
+ }
fmt.Fprintf(os.Stderr, ".")
case "testReadySlow":
ch, err := vexec.GetChildHandle()
diff --git a/runtimes/google/rt/mgmt.go b/runtimes/google/rt/mgmt.go
index ccaad62..56240d6 100644
--- a/runtimes/google/rt/mgmt.go
+++ b/runtimes/google/rt/mgmt.go
@@ -59,14 +59,7 @@
return err
}
- // Note(mattr): that we ignore the error here.
- // As far as I can tell this is because the modules framework actually calls
- // SetReady(), so we would then call it here a second time and get
- // an error.
- // TODO(mattr): We should change the modules framework so we can properly check
- // errors here.
- handle.SetReady()
- return nil
+ return handle.SetReady()
}
func getListenSpec(handle *exec.ChildHandle) (*ipc.ListenSpec, error) {
diff --git a/services/mgmt/application/applicationd/applicationd_v23_test.go b/services/mgmt/application/applicationd/applicationd_v23_test.go
index 2f521bd..e6c6a46 100644
--- a/services/mgmt/application/applicationd/applicationd_v23_test.go
+++ b/services/mgmt/application/applicationd/applicationd_v23_test.go
@@ -84,7 +84,7 @@
if err != nil {
i.Fatal(err)
}
- sigJSON, err := json.MarshalIndent(sig, " ", " ")
+ sigJSON, err := json.MarshalIndent(sig, " ", " ")
if err != nil {
i.Fatal(err)
}
@@ -99,8 +99,10 @@
wantEnvelope := `{
"Title": "title",
"Args": null,
- "Binary": "foo",
- "Signature": ` + string(sigJSON) + `,
+ "Binary": {
+ "File": "foo",
+ "Signature": ` + string(sigJSON) + `
+ },
"Publisher": ` + string(pubJSON) + `,
"Env": null,
"Packages": null
