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.
This commit is contained in:
James Bardin 2017-07-06 17:25:04 -04:00
parent ac9abf579f
commit 657932261b
1 changed files with 12 additions and 9 deletions

View File

@ -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
}