From 657932261be648fc902a3cc347da581e7dee98b5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 6 Jul 2017 17:25:04 -0400 Subject: [PATCH] Make sure shadow.closeWalker doesn't copy Mutexes The Close methods on shadow.Values require pointer receivers because they contain a sync.Mutex, but that value was being copied through Value.Interface by the closeWalker. Because reflectwalk passes the struct fields to the StructField method as they are defined in the struct, and they may have been read as a value, we can't immediately call Interface() to check the method set without possibly copying the internal mutex values. Use the Implements method to first check if we need to call Interface, and if it's not, then we can check if the value is addressable. Because of this use of reflection, we can't vet for the copying of these locks. The minimal amount of code in the Close method left us only with a race detected within the mutex itself, which leads to a stacktrace pointing to the runtime rather than our code. --- helper/shadow/closer.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/helper/shadow/closer.go b/helper/shadow/closer.go index 7edd5e75d..edc1e2a93 100644 --- a/helper/shadow/closer.go +++ b/helper/shadow/closer.go @@ -39,6 +39,8 @@ func (w *closeWalker) Struct(reflect.Value) error { return nil } +var closerType = reflect.TypeOf((*io.Closer)(nil)).Elem() + func (w *closeWalker) StructField(f reflect.StructField, v reflect.Value) error { // Not sure why this would be but lets avoid some panics if !v.IsValid() { @@ -56,17 +58,18 @@ func (w *closeWalker) StructField(f reflect.StructField, v reflect.Value) error return nil } - // We're looking for an io.Closer - raw := v.Interface() - if raw == nil { - return nil + var closer io.Closer + if v.Type().Implements(closerType) { + closer = v.Interface().(io.Closer) + } else if v.CanAddr() { + // The Close method may require a pointer receiver, but we only have a value. + v := v.Addr() + if v.Type().Implements(closerType) { + closer = v.Interface().(io.Closer) + } } - closer, ok := raw.(io.Closer) - if !ok && v.CanAddr() { - closer, ok = v.Addr().Interface().(io.Closer) - } - if !ok { + if closer == nil { return reflectwalk.SkipEntry }