Skip to content

Commit

Permalink
helper/shadow: Close for auto-closing all values
Browse files Browse the repository at this point in the history
Fixes #10122

The simple fix was that we forgot to close `ReadDataApply` for the
provider. But I've always felt that this section of the code was brittle
and I wanted to put in a more robust solution. The `shadow.Close` method
uses reflection to automatically close all values.
  • Loading branch information
mitchellh committed Nov 15, 2016
1 parent 7fbd880 commit 3c339ee
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 17 deletions.
81 changes: 81 additions & 0 deletions helper/shadow/closer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package shadow

import (
"fmt"
"io"
"reflect"

"github.com/hashicorp/go-multierror"
"github.com/mitchellh/reflectwalk"
)

// Close will close all shadow values within the given structure.
//
// This uses reflection to walk the structure, find all shadow elements,
// and close them. Currently this will only find struct fields that are
// shadow values, and not slice elements, etc.
func Close(v interface{}) error {
// We require a pointer so we can address the internal fields
val := reflect.ValueOf(v)
if val.Kind() != reflect.Ptr {
return fmt.Errorf("value must be a pointer")
}

// Walk and close
var w closeWalker
if err := reflectwalk.Walk(v, &w); err != nil {
return err
}

return w.Err
}

type closeWalker struct {
Err error
}

func (w *closeWalker) Struct(reflect.Value) error {
// Do nothing. We implement this for reflectwalk.StructWalker
return nil
}

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() {
return nil
}

// Empty for exported, so don't check unexported fields
if f.PkgPath != "" {
return nil
}

// Verify the io.Closer is in this package
typ := v.Type()
if typ.PkgPath() != "github.com/hashicorp/terraform/helper/shadow" {
return nil
}

// We're looking for an io.Closer
raw := v.Interface()
if raw == nil {
return nil
}

var closer io.Closer
closer, ok := raw.(io.Closer)
if !ok && v.CanAddr() {
closer, ok = v.Addr().Interface().(io.Closer)
}
if !ok {
return reflectwalk.SkipEntry
}

// Close it
if err := closer.Close(); err != nil {
w.Err = multierror.Append(w.Err, err)
}

// Don't go into the struct field
return reflectwalk.SkipEntry
}
69 changes: 69 additions & 0 deletions helper/shadow/closer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package shadow

import (
"testing"
"time"
)

func TestClose(t *testing.T) {
var foo struct {
A Value
B KeyedValue
}

if err := Close(&foo); err != nil {
t.Fatalf("err: %s", err)
}

if v := foo.A.Value(); v != ErrClosed {
t.Fatalf("bad: %#v", v)
}
if v := foo.B.Value("foo"); v != ErrClosed {
t.Fatalf("bad: %#v", v)
}
}

func TestClose_nonPtr(t *testing.T) {
var foo struct{}

if err := Close(foo); err == nil {
t.Fatal("should error")
}
}

func TestClose_unexported(t *testing.T) {
var foo struct {
A Value
b Value
}

if err := Close(&foo); err != nil {
t.Fatalf("err: %s", err)
}

if v := foo.A.Value(); v != ErrClosed {
t.Fatalf("bad: %#v", v)
}

// Start trying to get the value
valueCh := make(chan interface{})
go func() {
valueCh <- foo.b.Value()
}()

// We should not get the value
select {
case <-valueCh:
t.Fatal("shouldn't receive value")
case <-time.After(10 * time.Millisecond):
}

// Set the value
foo.b.Close()
val := <-valueCh

// Verify
if val != ErrClosed {
t.Fatalf("bad: %#v", val)
}
}
18 changes: 1 addition & 17 deletions terraform/shadow_resource_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package terraform

import (
"fmt"
"io"
"log"
"sync"

Expand Down Expand Up @@ -309,22 +308,7 @@ type shadowResourceProviderShared struct {
}

func (p *shadowResourceProviderShared) Close() error {
closers := []io.Closer{
&p.CloseErr, &p.Input, &p.Validate,
&p.Configure, &p.ValidateResource, &p.Apply, &p.Diff,
&p.Refresh, &p.ValidateDataSource, &p.ReadDataDiff,
}

for _, c := range closers {
// This should never happen, but we don't panic because a panic
// could affect the real behavior of Terraform and a shadow should
// never be able to do that.
if err := c.Close(); err != nil {
return err
}
}

return nil
return shadow.Close(p)
}

func (p *shadowResourceProviderShadow) CloseShadow() error {
Expand Down

0 comments on commit 3c339ee

Please sign in to comment.