Browspr fixes:
* Do cleanup() in a goroutine.
* Add lock around the activeInstances map.
* Remove references to "websocket" in comments and logs.
Change-Id: I86563e2398b94bf19857a6386db0cc58476c34b8
diff --git a/services/wsprd/app/app.go b/services/wsprd/app/app.go
index 43a56db..ed321a0 100644
--- a/services/wsprd/app/app.go
+++ b/services/wsprd/app/app.go
@@ -132,7 +132,7 @@
// A manager that Handles fetching and caching signature of remote services
signatureManager lib.SignatureManager
- // We maintain multiple Veyron server per websocket pipe for serving JavaScript
+ // We maintain multiple Veyron server per pipe for serving JavaScript
// services.
servers map[uint64]*server.Server
@@ -321,7 +321,7 @@
// Cleanup cleans up any outstanding rpcs.
func (c *Controller) Cleanup() {
- c.logger.VI(0).Info("Cleaning up websocket")
+ c.logger.VI(0).Info("Cleaning up pipe")
c.Lock()
defer c.Unlock()
@@ -521,7 +521,7 @@
}
func (c *Controller) serve(serveRequest serveRequest, w lib.ClientWriter) {
- // Create a server for the websocket pipe, if it does not exist already.
+ // Create a server for the pipe, if it does not exist already.
server, err := c.maybeCreateServer(serveRequest.ServerId)
if err != nil {
w.Error(verror2.Convert(verror2.Internal, nil, err))
@@ -607,7 +607,7 @@
return
}
- // Create a server for the websocket pipe, if it does not exist already
+ // Create a server for the pipe, if it does not exist already
server, err := c.maybeCreateServer(request.ServerId)
if err != nil {
w.Error(verror2.Convert(verror2.Internal, nil, err))
@@ -635,7 +635,7 @@
return
}
- // Create a server for the websocket pipe, if it does not exist already
+ // Create a server for the pipe, if it does not exist already
server, err := c.maybeCreateServer(request.ServerId)
if err != nil {
w.Error(verror2.Convert(verror2.Internal, nil, err))
diff --git a/services/wsprd/browspr/browspr.go b/services/wsprd/browspr/browspr.go
index f8f6f5e..163ac55 100644
--- a/services/wsprd/browspr/browspr.go
+++ b/services/wsprd/browspr/browspr.go
@@ -5,6 +5,7 @@
"fmt"
"net"
"regexp"
+ "sync"
"time"
"veyron.io/veyron/veyron2"
@@ -27,6 +28,8 @@
accountManager *account.AccountManager
postMessage func(instanceId int32, ty, msg string)
+ // Locks activeInstances
+ mu sync.Mutex
activeInstances map[int32]*pipe
}
@@ -98,6 +101,8 @@
// HandleMessage handles most messages from javascript and forwards them to a
// Controller.
func (b *Browspr) HandleMessage(instanceId int32, msg string) error {
+ b.mu.Lock()
+ defer b.mu.Unlock()
instance, ok := b.activeInstances[instanceId]
if !ok {
instance = newPipe(b, instanceId)
@@ -110,9 +115,15 @@
// HandleCleanupMessage cleans up the specified instance state. (For instance,
// when a browser tab is closed)
func (b *Browspr) HandleCleanupMessage(instanceId int32) {
+ b.mu.Lock()
+ defer b.mu.Unlock()
if instance, ok := b.activeInstances[instanceId]; ok {
- instance.cleanup()
delete(b.activeInstances, 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()
}
}
diff --git a/services/wsprd/browspr/main/main_nacl.go b/services/wsprd/browspr/main/main_nacl.go
index 890db0c..d313e76 100644
--- a/services/wsprd/browspr/main/main_nacl.go
+++ b/services/wsprd/browspr/main/main_nacl.go
@@ -19,11 +19,8 @@
"veyron.io/wspr/veyron/services/wsprd/browspr"
)
-func init() {
- stream.RegisterProtocol("ws", websocket.Dial, nil)
-}
-
func main() {
+ stream.RegisterProtocol("ws", websocket.Dial, nil)
ppapi.Init(newBrowsprInstance)
}
@@ -228,7 +225,7 @@
fmt.Printf("Starting browspr with config: proxy=%q mounttable=%q identityd=%q ", veyronProxy, mounttable, identityd)
inst.browspr = browspr.NewBrowspr(inst.BrowsprOutgoingPostMessage, chrome.New, listenSpec, identityd, []string{mounttable}, options.RuntimePrincipal{principal})
- inst.BrowsprOutgoingPostMessage(instanceId, "browsprStarted", "blah")
+ inst.BrowsprOutgoingPostMessage(instanceId, "browsprStarted", "")
return nil
}
@@ -316,7 +313,6 @@
// A message is of the form {"type": "typeName", "body": { stuff here }},
// where the body is passed to the message handler.
func (inst *browsprInstance) HandleMessage(message ppapi.Var) {
- fmt.Printf("Entered HandleMessage")
instanceId, err := message.LookupIntValuedKey("instanceId")
if err != nil {
inst.handleGoError(err)
diff --git a/services/wsprd/browspr/pipe.go b/services/wsprd/browspr/pipe.go
index c0f6910..5c06ac3 100644
--- a/services/wsprd/browspr/pipe.go
+++ b/services/wsprd/browspr/pipe.go
@@ -52,7 +52,7 @@
}
func (p *pipe) cleanup() {
- p.browspr.logger.VI(0).Info("Cleaning up websocket")
+ p.browspr.logger.VI(0).Info("Cleaning up controller")
p.controller.Cleanup()
}
