aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorColin Cross2019-05-14 13:33:05 -0500
committerColin Cross2019-05-14 13:40:13 -0500
commit66bdb695ae6d1a8df84d151b1aa2a30b5e52a52e (patch)
treeb345e9626032fde682124bc1ea909b17b2722e5b
parente4948c79d38d91a3678c98e804b27de37e18adbf (diff)
downloadplatform-build-soong-66bdb695ae6d1a8df84d151b1aa2a30b5e52a52e.tar.gz
platform-build-soong-66bdb695ae6d1a8df84d151b1aa2a30b5e52a52e.tar.xz
platform-build-soong-66bdb695ae6d1a8df84d151b1aa2a30b5e52a52e.zip
Prevent hangs in OncePer when the callback panics
If the callback passed to Once panics it was leaving the waiter in place that would never be completed. Move writing the value and signalling the waiter to defer. Test: TestOncePerPanic Change-Id: Icc4d3b779a79914fcd881d61d38dffcc2f591c39
-rw-r--r--android/onceper.go16
-rw-r--r--android/onceper_test.go40
2 files changed, 51 insertions, 5 deletions
diff --git a/android/onceper.go b/android/onceper.go
index 5ad17fa9..ff865c2e 100644
--- a/android/onceper.go
+++ b/android/onceper.go
@@ -40,7 +40,8 @@ func (once *OncePer) maybeWaitFor(key OnceKey, value interface{}) interface{} {
40} 40}
41 41
42// Once computes a value the first time it is called with a given key per OncePer, and returns the 42// Once computes a value the first time it is called with a given key per OncePer, and returns the
43// value without recomputing when called with the same key. key must be hashable. 43// value without recomputing when called with the same key. key must be hashable. If value panics
44// the panic will be propagated but the next call to Once with the same key will return nil.
44func (once *OncePer) Once(key OnceKey, value func() interface{}) interface{} { 45func (once *OncePer) Once(key OnceKey, value func() interface{}) interface{} {
45 // Fast path: check if the key is already in the map 46 // Fast path: check if the key is already in the map
46 if v, ok := once.values.Load(key); ok { 47 if v, ok := once.values.Load(key); ok {
@@ -54,10 +55,15 @@ func (once *OncePer) Once(key OnceKey, value func() interface{}) interface{} {
54 return once.maybeWaitFor(key, v) 55 return once.maybeWaitFor(key, v)
55 } 56 }
56 57
57 // The waiter is inserted, call the value constructor, store it, and signal the waiter 58 // The waiter is inserted, call the value constructor, store it, and signal the waiter. Use defer in case
58 v := value() 59 // the function panics.
59 once.values.Store(key, v) 60 var v interface{}
60 close(waiter) 61 defer func() {
62 once.values.Store(key, v)
63 close(waiter)
64 }()
65
66 v = value()
61 67
62 return v 68 return v
63} 69}
diff --git a/android/onceper_test.go b/android/onceper_test.go
index 95303baf..1a55ff4b 100644
--- a/android/onceper_test.go
+++ b/android/onceper_test.go
@@ -175,3 +175,43 @@ func TestOncePerReentrant(t *testing.T) {
175 t.Errorf(`reentrant Once should return "a": %q`, a) 175 t.Errorf(`reentrant Once should return "a": %q`, a)
176 } 176 }
177} 177}
178
179// Test that a recovered panic in a Once function doesn't deadlock
180func TestOncePerPanic(t *testing.T) {
181 once := OncePer{}
182 key := NewOnceKey("key")
183
184 ch := make(chan interface{})
185
186 var a interface{}
187
188 go func() {
189 defer func() {
190 ch <- recover()
191 }()
192
193 a = once.Once(key, func() interface{} {
194 panic("foo")
195 })
196 }()
197
198 p := <-ch
199
200 if p.(string) != "foo" {
201 t.Errorf(`expected panic with "foo", got %#v`, p)
202 }
203
204 if a != nil {
205 t.Errorf(`expected a to be nil, got %#v`, a)
206 }
207
208 // If the call to Once that panicked leaves the key in a bad state this will deadlock
209 b := once.Once(key, func() interface{} {
210 return "bar"
211 })
212
213 // The second call to Once should return nil inserted by the first call that panicked.
214 if b != nil {
215 t.Errorf(`expected b to be nil, got %#v`, b)
216 }
217}