veyron/runtimes/google/ipc/stream/vif: Handle caching of VIFs over unix sockets correctly.
By not caching them at all.
With net.Conns on Unix sockets, distinct unix sockets do not end up
with distinct net.Conn.RemoteAddr (the "address" field is empty).
Avoid using the cache for these cases.
Added unittests that fail without the fix to set.go.
Change-Id: I1373398a05e0d5efc2db18f82e94f1c21b7d9e6e
diff --git a/runtimes/google/ipc/stream/vif/set.go b/runtimes/google/ipc/stream/vif/set.go
index c4b28ce..6ecbe60 100644
--- a/runtimes/google/ipc/stream/vif/set.go
+++ b/runtimes/google/ipc/stream/vif/set.go
@@ -25,6 +25,14 @@
// If there are multiple VIFs established to the same remote network address,
// Find will randomly return one of them.
func (s *Set) Find(network, address string) *VIF {
+ if len(address) == 0 || (network == "pipe" && address == "pipe") {
+ // Some network connections (like those created with net.Pipe
+ // or Unix sockets) do not end up with distinct conn.RemoteAddrs
+ // on distinct net.Conns. For those cases, avoid the cache collisions
+ // by disabling cache lookups for them.
+ return nil
+ }
+
s.mu.RLock()
defer s.mu.RUnlock()
l, ok := s.set[s.key(network, address)]
diff --git a/runtimes/google/ipc/stream/vif/set_test.go b/runtimes/google/ipc/stream/vif/set_test.go
new file mode 100644
index 0000000..e704cb3
--- /dev/null
+++ b/runtimes/google/ipc/stream/vif/set_test.go
@@ -0,0 +1,84 @@
+package vif_test
+
+import (
+ "io/ioutil"
+ "net"
+ "path"
+ "testing"
+
+ "veyron2/naming"
+
+ "veyron/runtimes/google/ipc/stream/vif"
+)
+
+func TestSetWithPipes(t *testing.T) {
+ var (
+ conn1, _ = net.Pipe()
+ conn2, _ = net.Pipe()
+ addr1 = conn1.RemoteAddr()
+ addr2 = conn2.RemoteAddr()
+
+ vf, err = vif.InternalNewDialedVIF(conn1, naming.FixedRoutingID(1), nil)
+ set = vif.NewSet()
+ )
+ if err != nil {
+ t.Fatal(err)
+ }
+ if addr1.Network() != addr2.Network() || addr1.String() != addr2.String() {
+ t.Fatalf("This test was intended for distinct connections that have duplicate RemoteAddrs. "+
+ "That does not seem to be the case with (%q, %q) and (%q, %q)",
+ addr1.Network(), addr1, addr2.Network(), addr2)
+ }
+ set.Insert(vf)
+ if found := set.Find(addr2.Network(), addr2.String()); found != nil {
+ t.Fatalf("Got [%v] want nil on Find(%q, %q))", found, addr2.Network(), addr2)
+ }
+}
+
+func TestSetWithUnixSocket(t *testing.T) {
+ dir, err := ioutil.TempDir("", "TestSetWithFileConn")
+ if err != nil {
+ t.Fatal(err)
+ }
+ sockname := path.Join(dir, "socket")
+ ln, err := net.Listen("unix", sockname)
+ if err != nil {
+ t.Fatal(err)
+ }
+ // Setup the creation of two separate connections.
+ // At the listener, they will end up with two distinct connections
+ // with the same "address" (empty string).
+ go func() {
+ for i := 0; i < 2; i++ {
+ if _, err := net.Dial("unix", sockname); err != nil {
+ ln.Close()
+ t.Fatal(err)
+ }
+ }
+ }()
+
+ conn1, err := ln.Accept()
+ if err != nil {
+ t.Fatal(err)
+ }
+ conn2, err := ln.Accept()
+ if err != nil {
+ t.Fatal(err)
+ }
+ addr1 := conn1.RemoteAddr()
+ addr2 := conn2.RemoteAddr()
+ if addr1.Network() != addr2.Network() || addr1.String() != addr2.String() {
+ t.Fatalf("This test was intended for distinct connections that have duplicate RemoteAddrs. "+
+ "That does not seem to be the case with (%q, %q) and (%q, %q)",
+ addr1.Network(), addr1, addr2.Network(), addr2)
+ }
+ vif1, err := vif.InternalNewAcceptedVIF(conn1, naming.FixedRoutingID(1), nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ set := vif.NewSet()
+ set.Insert(vif1)
+ if found := set.Find(addr2.Network(), addr2.String()); found != nil {
+ t.Errorf("Got [%v] want nil on Find(%q, %q)", found, addr2.Network(), addr2)
+ }
+}
