bugfix: race conditions in Future methods

Before the patch it may be possible to close several times fut.ready
and fut.done channels from the public API calls.

Author: oleg-jukovec

CHANGELOG.md

@@ -33,6 +33,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 
 - Build with OpenSSL < 1.1.1 (#194)
 - Add `ExecuteAsync` and `ExecuteTyped` to common connector interface (#62)
+- Race conditions in methods of `Future` type (#195)
 
 ## [1.6.0] - 2022-06-01
 

future.go

@@ -129,38 +129,41 @@ func NewFuture() (fut *Future) {
 // AppendPush appends the push response to the future.
 // Note: it works only before SetResponse() or SetError()
 func (fut *Future) AppendPush(resp *Response) {
+	fut.mutex.Lock()
+	defer fut.mutex.Unlock()
+
 	if fut.isDone() {
 		return
 	}
 	resp.Code = PushCode
-	fut.mutex.Lock()
 	fut.pushes = append(fut.pushes, resp)
-	fut.mutex.Unlock()
 
 	fut.ready <- struct{}{}
 }
 
 // SetResponse sets a response for the future and finishes the future.
 func (fut *Future) SetResponse(resp *Response) {
+	fut.mutex.Lock()
+	defer fut.mutex.Unlock()
+
 	if fut.isDone() {
 		return
 	}
-	fut.mutex.Lock()
 	fut.resp = resp
-	fut.mutex.Unlock()
 
 	close(fut.ready)
 	close(fut.done)
 }
 
 // SetError sets an error for the future and finishes the future.
 func (fut *Future) SetError(err error) {
+	fut.mutex.Lock()
+	defer fut.mutex.Unlock()
+
 	if fut.isDone() {
 		return
 	}
-	fut.mutex.Lock()
 	fut.err = err
-	fut.mutex.Unlock()
 
 	close(fut.ready)
 	close(fut.done)

future_test.go

@@ -233,3 +233,26 @@ func TestFutureGetIteratorError(t *testing.T) {
 		}
 	}
 }
+
+func TestFutureSetStateRaceCondition(t *testing.T) {
+	err := errors.New("any error")
+	resp := &Response{}
+	respAppend := &Response{}
+
+	for i := 0; i < 1000; i++ {
+		fut := NewFuture()
+		for j := 0; j < 9; j++ {
+			go func(opt int) {
+				if opt%3 == 0 {
+					fut.AppendPush(respAppend)
+				} else if opt%3 == 1 {
+					fut.SetError(err)
+				} else {
+					fut.SetResponse(resp)
+				}
+			}(j)
+		}
+	}
+	// It may be false-positive, but very rarely - it's ok for such very
+	// simple race conditions tests.
+}