From 6557a3de18ece4ab2780b9ae300c28b0fec43ed0 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 15 Nov 2016 08:45:45 -0800
Subject: [PATCH] helper/shadow: Close for auto-closing all values

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.
---
 helper/shadow/closer.go               | 80 +++++++++++++++++++++++++++
 helper/shadow/closer_test.go          | 69 +++++++++++++++++++++++
 terraform/shadow_resource_provider.go | 18 +-----
 3 files changed, 150 insertions(+), 17 deletions(-)
 create mode 100644 helper/shadow/closer.go
 create mode 100644 helper/shadow/closer_test.go

diff --git a/helper/shadow/closer.go b/helper/shadow/closer.go
new file mode 100644
index 000000000..7edd5e75d
--- /dev/null
+++ b/helper/shadow/closer.go
@@ -0,0 +1,80 @@
+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
+	}
+
+	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
+}
diff --git a/helper/shadow/closer_test.go b/helper/shadow/closer_test.go
new file mode 100644
index 000000000..83e744c54
--- /dev/null
+++ b/helper/shadow/closer_test.go
@@ -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)
+	}
+}
diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go
index 6084db615..4976fdc66 100644
--- a/terraform/shadow_resource_provider.go
+++ b/terraform/shadow_resource_provider.go
@@ -2,7 +2,6 @@ package terraform
 
 import (
 	"fmt"
-	"io"
 	"log"
 	"sync"
 
@@ -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 {