[fidl] Go bindings: Make Bindings private
Final part of soft transition to prevent race condition iterating over
BindingSet bindings.
Soft transition consists of:
1) Exposing new method in BindingSet to allow safe iteration over keys.
2) Update all clients to use new method instead of public field
Bindings.
3) Make field Bindings private to prevent future concurrency problems.
BUG: FIDL-435
TEST: CQ
Change-Id: I41670a7f09844fd54f8c97fc00e728c1c66605c3
diff --git a/src/syscall/zx/fidl/bindings.go b/src/syscall/zx/fidl/bindings.go
index c26b0b0..b330cea 100644
--- a/src/syscall/zx/fidl/bindings.go
+++ b/src/syscall/zx/fidl/bindings.go
@@ -221,9 +221,9 @@
// BindingSet is a managed set of Bindings which know how to unbind and
// remove themselves in the event of a connection error.
type BindingSet struct {
- nextKey BindingKey
mu sync.Mutex
- Bindings map[BindingKey]*Binding
+ nextKey BindingKey
+ bindings map[BindingKey]*Binding
}
// Add creates a new Binding, initializes it, and adds it to the set.
@@ -231,14 +231,14 @@
// onError is an optional handler than may be passed which will be called after
// the binding between the Stub and the Channel is successfully closed.
func (b *BindingSet) Add(s Stub, c zx.Channel, onError func(error)) (BindingKey, error) {
+ b.mu.Lock()
+ defer b.mu.Unlock()
binding := &Binding{
Stub: s,
Channel: c,
}
- b.mu.Lock()
- defer b.mu.Unlock()
- if b.Bindings == nil {
- b.Bindings = make(map[BindingKey]*Binding)
+ if b.bindings == nil {
+ b.bindings = make(map[BindingKey]*Binding)
}
key := b.nextKey
err := binding.Init(func(err error) {
@@ -249,7 +249,7 @@
if err != nil {
return 0, err
}
- b.Bindings[key] = binding
+ b.bindings[key] = binding
b.nextKey += 1
return key, nil
}
@@ -258,8 +258,8 @@
func (b *BindingSet) BindingKeys() []BindingKey {
b.mu.Lock()
defer b.mu.Unlock()
- r := make([]BindingKey, 0, len(b.Bindings))
- for key := range b.Bindings {
+ r := make([]BindingKey, 0, len(b.bindings))
+ for key := range b.bindings {
r = append(r, key)
}
return r
@@ -270,7 +270,7 @@
func (b *BindingSet) ProxyFor(key BindingKey) (*ChannelProxy, bool) {
b.mu.Lock()
defer b.mu.Unlock()
- if binding, ok := b.Bindings[key]; ok {
+ if binding, ok := b.bindings[key]; ok {
return &ChannelProxy{Channel: binding.Channel}, true
}
return nil, false
@@ -284,9 +284,9 @@
// Returns true if a Binding was found and removed.
func (b *BindingSet) Remove(key BindingKey) bool {
b.mu.Lock()
- if binding, ok := b.Bindings[key]; ok {
- delete(b.Bindings, key)
- b.mu.Unlock()
+ defer b.mu.Unlock()
+ if binding, ok := b.bindings[key]; ok {
+ delete(b.bindings, key)
// Close the binding before calling the callback.
if err := binding.Close(); err != nil {
@@ -297,7 +297,6 @@
}
return true
}
- b.mu.Unlock()
return false
}
@@ -305,7 +304,8 @@
func (b *BindingSet) Close() {
// Lock, close all the bindings, and clear the map.
b.mu.Lock()
- for _, binding := range b.Bindings {
+ defer b.mu.Unlock()
+ for _, binding := range b.bindings {
if err := binding.Close(); err != nil {
// Just panic. The only reason this can fail is if the handle
// is bad, which it shouldn't be if we're tracking things. If
@@ -313,6 +313,5 @@
panic(prefix + err.Error())
}
}
- b.Bindings = make(map[BindingKey]*Binding)
- b.mu.Unlock()
+ b.bindings = nil
}
