From e10a7917e6c85fbb0e20380101ce8a8f27e81c89 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 19 Mar 2018 17:09:53 -0400
Subject: [PATCH 1/3] add failing test for windows state locks

Refreshing a locked state on windows could return nil if the read path
was locked, no state was yet written, and the read path is the same as
the write path.

Add a test that locks then refreshes a newly initialized state struct.
---
 state/local_test.go | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/state/local_test.go b/state/local_test.go
index 633356028..13dcf6eab 100644
--- a/state/local_test.go
+++ b/state/local_test.go
@@ -166,3 +166,42 @@ func testLocalState(t *testing.T) *LocalState {
 
 	return ls
 }
+
+// Make sure we can refresh while the state is locked
+func TestLocalState_refreshWhileLocked(t *testing.T) {
+	f, err := ioutil.TempFile("", "tf")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	err = terraform.WriteState(TestStateInitial(), f)
+	f.Close()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	s := &LocalState{Path: f.Name()}
+	defer os.Remove(s.Path)
+
+	// lock first
+	info := NewLockInfo()
+	info.Operation = "test"
+	lockID, err := s.Lock(info)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := s.Unlock(lockID); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	if err := s.RefreshState(); err != nil {
+		t.Fatal(err)
+	}
+
+	readState := s.State()
+	if readState == nil || readState.Lineage == "" {
+		t.Fatal("missing state")
+	}
+}

From 8fb8b2cffc54e4e724dffb1afb45219152c49fda Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 19 Mar 2018 18:15:54 -0400
Subject: [PATCH 2/3] make sure ReadState returns an error

ReadState would hide any errors, assuming that it was an empty state.
This can mask errors on Windows, where the OS enforces read locks on the
state file.
---
 terraform/state.go | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/terraform/state.go b/terraform/state.go
index 89203bbfe..04b14a659 100644
--- a/terraform/state.go
+++ b/terraform/state.go
@@ -9,6 +9,7 @@ import (
 	"io"
 	"io/ioutil"
 	"log"
+	"os"
 	"reflect"
 	"sort"
 	"strconv"
@@ -1876,13 +1877,21 @@ var ErrNoState = errors.New("no state")
 // ReadState reads a state structure out of a reader in the format that
 // was written by WriteState.
 func ReadState(src io.Reader) (*State, error) {
-	buf := bufio.NewReader(src)
-	if _, err := buf.Peek(1); err != nil {
-		// the error is either io.EOF or "invalid argument", and both are from
-		// an empty state.
+	// check for a nil file specifically, since that produces a platform
+	// specific error if we try to use it in a bufio.Reader.
+	if f, ok := src.(*os.File); ok && f == nil {
 		return nil, ErrNoState
 	}
 
+	buf := bufio.NewReader(src)
+
+	if _, err := buf.Peek(1); err != nil {
+		if err == io.EOF {
+			return nil, ErrNoState
+		}
+		return nil, err
+	}
+
 	if err := testForV0State(buf); err != nil {
 		return nil, err
 	}

From a84b4a669ae053b17f37d1acab4d65daa82d2591 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 19 Mar 2018 17:19:50 -0400
Subject: [PATCH 3/3] use the open state file for refresh when possible

Only open a new file descriptor for RefreshState if we haven't written a
state, and don't have the correct state open already. This prevents
windows from failing to refresh a locked state.
---
 state/local.go | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/state/local.go b/state/local.go
index a6d17653b..609881f72 100644
--- a/state/local.go
+++ b/state/local.go
@@ -119,8 +119,20 @@ func (s *LocalState) RefreshState() error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
+	if s.PathOut == "" {
+		s.PathOut = s.Path
+	}
+
 	var reader io.Reader
-	if !s.written {
+
+	// The s.Path file is only OK to read if we have not written any state out
+	// (in which case the same state needs to be read in), and no state output file
+	// has been opened (possibly via a lock) or the input path is different
+	// than the output path.
+	// This is important for Windows, as if the input file is the same as the
+	// output file, and the output file has been locked already, we can't open
+	// the file again.
+	if !s.written && (s.stateFileOut == nil || s.Path != s.PathOut) {
 		// we haven't written a state file yet, so load from Path
 		f, err := os.Open(s.Path)
 		if err != nil {