rpc/stream/vif: fix flaky failure in IdleTimeout test
Currently there are two races in a heavy load environment;
o fake timers can be triggered before test setup - i.e.,
before creating flows for testing.
- Fixed by turn the fake timers after creating flows
o when triggering a fake timer, we create a real timer
and then stop it if the fake timer is already stopped.
This is because no way to create a stopped timer.
But the timer can be triggered before it is stopped
with a heavy load.
- Fixed by not creating a real timer if it is stopped
and create one when it is requested (by Reset()).
o Changed some t.Error to t.Fatal, since it makes debugging
hard by consuming unexpected notifications and causing
test timeout failure in the following test.
Change-Id: I24962f253fae00d32c18159681199da7d3f68bc3
diff --git a/runtime/internal/rpc/stream/vif/faketimer.go b/runtime/internal/rpc/stream/vif/faketimer.go
index 914c4e2..d257a85 100644
--- a/runtime/internal/rpc/stream/vif/faketimer.go
+++ b/runtime/internal/rpc/stream/vif/faketimer.go
@@ -37,9 +37,15 @@
func (t *fakeTimer) Reset(d time.Duration) bool {
t.mu.Lock()
defer t.mu.Unlock()
+ active := t.timer.Stop()
t.timeout = d
t.stopped = false
- return t.timer.Reset(t.timeout)
+ if t.timeout > 0 {
+ t.timer = newTimer(t.timeout, t.timeoutFunc)
+ } else {
+ t.timer = noopTimer{}
+ }
+ return active
}
func (t *fakeTimer) run(release <-chan struct{}, wg *sync.WaitGroup) {
@@ -47,12 +53,9 @@
<-release // Wait until notified to run.
t.mu.Lock()
defer t.mu.Unlock()
- if t.timeout > 0 {
+ if t.timeout > 0 && !t.stopped {
t.timer = newTimer(t.timeout, t.timeoutFunc)
}
- if t.stopped {
- t.timer.Stop()
- }
}
// SetFakeTimers causes the idle timers to use a fake timer instead of one
diff --git a/runtime/internal/rpc/stream/vif/vif_test.go b/runtime/internal/rpc/stream/vif/vif_test.go
index 101258b..9d53964 100644
--- a/runtime/internal/rpc/stream/vif/vif_test.go
+++ b/runtime/internal/rpc/stream/vif/vif_test.go
@@ -476,9 +476,8 @@
}
return
}
- newVC := func(vf, remote *vif.VIF) (VC stream.VC, ln stream.Listener, remoteVC stream.Connector) {
- triggerTimers := vif.SetFakeTimers()
- defer triggerTimers()
+ newVC := func(vf, remote *vif.VIF) (VC stream.VC, ln stream.Listener, remoteVC stream.Connector, triggerTimers func()) {
+ triggerTimers = vif.SetFakeTimers()
var err error
VC, remoteVC, err = createVC(vf, remote, pclient, makeEP(0x10), vc.IdleTimeout{idleTime})
if err != nil {
@@ -500,14 +499,15 @@
// No active flow. Should be notified.
vf, remote := newVIF()
- _, _, _ = newVC(vf, remote)
+ _, _, _, triggerTimers := newVC(vf, remote)
+ triggerTimers()
if err := vif.WaitForNotifications(notify, vf, remote); err != nil {
t.Error(err)
}
// Same as above, but with multiple VCs.
vf, remote = newVIF()
- triggerTimers := vif.SetFakeTimers()
+ triggerTimers = vif.SetFakeTimers()
if _, _, err := createNVCs(vf, remote, pclient, 0x10, 5, vc.IdleTimeout{idleTime}); err != nil {
t.Fatal(err)
}
@@ -518,10 +518,11 @@
// Open one flow. Should not be notified.
vf, remote = newVIF()
- vc, _, _ := newVC(vf, remote)
+ vc, _, _, triggerTimers := newVC(vf, remote)
f1 := newFlow(vc, remote)
+ triggerTimers()
if err := vif.WaitWithTimeout(notify, waitTime); err != nil {
- t.Error(err)
+ t.Fatal(err)
}
// Close the flow. Should be notified.
@@ -532,17 +533,17 @@
// Open two flows.
vf, remote = newVIF()
- vc, _, _ = newVC(vf, remote)
+ vc, _, _, triggerTimers = newVC(vf, remote)
f1 = newFlow(vc, remote)
f2 := newFlow(vc, remote)
+ triggerTimers()
// Close the first flow twice. Should not be notified.
f1.Close()
f1.Close()
if err := vif.WaitWithTimeout(notify, waitTime); err != nil {
- t.Error(err)
+ t.Fatal(err)
}
-
// Close the second flow. Should be notified now.
f2.Close()
if err := vif.WaitForNotifications(notify, vf, remote); err != nil {
@@ -551,14 +552,15 @@
// Same as above, but open a flow from the remote side.
vf, remote = newVIF()
- _, ln, remoteVC := newVC(vf, remote)
+ _, ln, remoteVC, triggerTimers := newVC(vf, remote)
f1, err := remoteVC.Connect()
if err != nil {
t.Fatal(err)
}
acceptFlowAtClient(ln)
+ triggerTimers()
if err := vif.WaitWithTimeout(notify, waitTime); err != nil {
- t.Error(err)
+ t.Fatal(err)
}
f1.Close()
if err := vif.WaitForNotifications(notify, vf, remote); err != nil {
