Replace formerly necessary goroutine with more judicious use of mutexes.
Also fix some comments.
Change-Id: Ic6e192af27efc3351833f48c45e16d28fd551cc6
diff --git a/services/wsprd/app/app.go b/services/wsprd/app/app.go
index 10a1d91..62bf11b 100644
--- a/services/wsprd/app/app.go
+++ b/services/wsprd/app/app.go
@@ -281,7 +281,7 @@
// Cleanup cleans up any outstanding rpcs.
func (c *Controller) Cleanup() {
- c.GetLogger().VI(0).Info("Cleaning up pipe")
+ c.GetLogger().VI(0).Info("Cleaning up controller")
c.Lock()
defer c.Unlock()
diff --git a/services/wsprd/browspr/browspr.go b/services/wsprd/browspr/browspr.go
index 3328447..85274da 100644
--- a/services/wsprd/browspr/browspr.go
+++ b/services/wsprd/browspr/browspr.go
@@ -83,7 +83,6 @@
// Controller.
func (b *Browspr) HandleMessage(instanceId int32, origin, msg string) error {
b.mu.Lock()
- defer b.mu.Unlock()
instance, ok := b.activeInstances[instanceId]
if !ok {
instance = newPipe(b, instanceId, origin)
@@ -92,6 +91,7 @@
}
b.activeInstances[instanceId] = instance
}
+ b.mu.Unlock()
return instance.handleMessage(msg)
}
@@ -109,15 +109,16 @@
}
b.mu.Lock()
- defer b.mu.Unlock()
if instance, ok := b.activeInstances[msg.InstanceId]; ok {
delete(b.activeInstances, msg.InstanceId)
- // NOTE(nlacasse): Calling cleanup() on the main thread locks
- // browspr, so we must do it in a goroutine.
- // TODO(nlacasse): Consider running all the message handlers in
- // goroutines.
- go instance.cleanup()
+ // We must unlock the mutex before calling cleanunp, otherwise
+ // browspr deadlocks.
+ b.mu.Unlock()
+ instance.cleanup()
+ } else {
+ b.mu.Unlock()
}
+
return nil, nil
}
diff --git a/services/wsprd/browspr/pipe.go b/services/wsprd/browspr/pipe.go
index fbcc10f..b7e1a33 100644
--- a/services/wsprd/browspr/pipe.go
+++ b/services/wsprd/browspr/pipe.go
@@ -70,7 +70,7 @@
}
func (p *pipe) cleanup() {
- p.browspr.logger.VI(0).Info("Cleaning up controller")
+ p.browspr.logger.VI(0).Info("Cleaning up pipe")
p.controller.Cleanup()
}