Merge "veyron/runtimes/google/ipc: crude fixes for https://code.google.com/p/envyor/issues/detail?id=257 Addresses issues 1 and 2 in https://docs.google.com/a/google.com/document/d/1C0kxfYhuOcStdV7tnLZELZpUhfQCZj47B0JrzbE29h8/edit See doc for details."
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index ebabe5a..5027366 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -433,7 +433,20 @@
return nil
}
if err := fc.enc.Encode(ipc.Request{EndStreamArgs: true}); err != nil {
- return fc.close(verror.BadProtocolf("ipc: end stream args encoding failed: %v", err))
+ // TODO(caprita): Indiscriminately closing the flow below causes
+ // a race as described in:
+ // https://docs.google.com/a/google.com/document/d/1C0kxfYhuOcStdV7tnLZELZpUhfQCZj47B0JrzbE29h8/edit
+ //
+ // There should be a finer grained way to fix this (for example,
+ // encoding errors should probably still result in closing the
+ // flow); on the flip side, there may exist other instances
+ // where we are closing the flow but should not.
+ //
+ // For now, commenting out the line below removes the flakiness
+ // from our existing unit tests, but this needs to be revisited
+ // and fixed correctly.
+ //
+ // return fc.close(verror.BadProtocolf("ipc: end stream args encoding failed: %v", err))
}
fc.sendClosed = true
return nil
diff --git a/runtimes/google/ipc/stream/vif/vif.go b/runtimes/google/ipc/stream/vif/vif.go
index d487ee4..06d27bd 100644
--- a/runtimes/google/ipc/stream/vif/vif.go
+++ b/runtimes/google/ipc/stream/vif/vif.go
@@ -514,7 +514,20 @@
err = message.WriteTo(vif.conn, m)
}
if err != nil {
- vif.closeVCAndSendMsg(vc, fmt.Sprintf("write failure: %v", err))
+ // TODO(caprita): Calling closeVCAndSendMsg below causes
+ // a race as described in:
+ // https://docs.google.com/a/google.com/document/d/1C0kxfYhuOcStdV7tnLZELZpUhfQCZj47B0JrzbE29h8/edit
+ //
+ // There should be a finer grained way to fix this, and
+ // there are likely other instances where we should not
+ // be closing the VC.
+ //
+ // For now, commenting out the line below removes the
+ // flakiness from our existing unit tests, but this
+ // needs to be revisited and fixed correctly.
+ //
+ // vif.closeVCAndSendMsg(vc, fmt.Sprintf("write failure: %v", err))
+
// Drain the queue and exit.
for {
qm, err := messages.Get(nil)
