runtimes/google/ipc/server.go: fix a race.
- it's only safe to close the publisher's channel if CloseFork
doesn't report an error. In general, the only error CloseFork
will return is that the channel/publisher has already been closed!
Change-Id: I734707205a045091e321fc38d2cb99090e5a759d
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 1cf87e2..e4f5c12 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -856,8 +856,13 @@
}
if dhcp := s.dhcpState; dhcp != nil {
- dhcp.publisher.CloseFork(dhcp.name, dhcp.ch)
- drain(dhcp.ch)
+ // TODO(cnicolaou,caprita): investigate not having to close and drain
+ // the channel here. It's a little awkward right now since we have to
+ // be careful to not close the channel in two places, i.e. here and
+ // and from the publisher's Shutdown method.
+ if err := dhcp.publisher.CloseFork(dhcp.name, dhcp.ch); err == nil {
+ drain(dhcp.ch)
+ }
}
s.Unlock()
diff --git a/runtimes/google/ipc/server_test.go b/runtimes/google/ipc/server_test.go
index 728a4bf..5c52190 100644
--- a/runtimes/google/ipc/server_test.go
+++ b/runtimes/google/ipc/server_test.go
@@ -420,7 +420,7 @@
publisher := config.NewPublisher()
roaming := make(chan config.Setting)
- stop, err := publisher.CreateStream("roaming", "roaming", roaming)
+ stop, err := publisher.CreateStream("TestRoaming", "TestRoaming", roaming)
if err != nil {
t.Fatal(err)
}
@@ -438,7 +438,7 @@
{"tcp", ":0"},
{"tcp", ":0"},
},
- StreamName: "roaming",
+ StreamName: "TestRoaming",
StreamPublisher: publisher,
AddressChooser: ipv4And6,
}
@@ -553,6 +553,7 @@
if got, want := len(change.Changed), 2; got != want {
t.Fatalf("got %d, want %d", got, want)
}
+
}
func TestWatcherDeadlock(t *testing.T) {
@@ -568,7 +569,7 @@
publisher := config.NewPublisher()
roaming := make(chan config.Setting)
- stop, err := publisher.CreateStream("roaming", "roaming", roaming)
+ stop, err := publisher.CreateStream("TestWatcherDeadlock", "TestWatcherDeadlock", roaming)
if err != nil {
t.Fatal(err)
}
@@ -578,7 +579,7 @@
Addrs: ipc.ListenAddrs{
{"tcp", ":0"},
},
- StreamName: "roaming",
+ StreamName: "TestWatcherDeadlock",
StreamPublisher: publisher,
}
eps, err := server.Listen(spec)