From 97ae8e0db7bf48a12a724e3c8d482de7c16e8fcb Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Wed, 22 Apr 2020 15:17:25 -0500
Subject: [PATCH] add kuba testcase, fix race

we fix the race by not returning pointers to the things which we're
keeping in the in-memory store
---
 server/server_test.go | 20 +++++++++++++++++++-
 transaction.go        | 19 +++++++++++++++----
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/server/server_test.go b/server/server_test.go
index d84dbe50c..d7cb9452e 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -36,6 +36,7 @@ import (
 	"github.com/pilosa/pilosa/v2/roaring"
 	"github.com/pilosa/pilosa/v2/server"
 	"github.com/pilosa/pilosa/v2/test"
+	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -468,10 +469,27 @@ func TestTransactionsAPI(t *testing.T) {
 	}
 
 	// can poll exclusive transaction and is active
+	var excTrns *pilosa.Transaction
 	if trns, err := api0.GetTransaction(ctx, "exc", false); err != nil {
 		t.Errorf("couldn't poll exclusive transaction: %v", err)
 	} else {
-		test.CompareTransactions(t, &pilosa.Transaction{ID: "exc", Active: true, Exclusive: true, Timeout: time.Minute, Deadline: time.Now().Add(time.Minute)}, trns)
+		excTrns = &pilosa.Transaction{ID: "exc", Active: true, Exclusive: true, Timeout: time.Minute, Deadline: time.Now().Add(time.Minute)}
+		test.CompareTransactions(t, excTrns, trns)
+	}
+
+	// can't start another exclusive transaction
+	if trns, err := api0.StartTransaction(ctx, "exc2", time.Minute, true, false); errors.Cause(err) != pilosa.ErrTransactionExclusive {
+		t.Errorf("unexpected error: %v", err)
+	} else {
+		// returned transaction should be the exclusive one which is blocking this one
+		test.CompareTransactions(t, excTrns, trns)
+	}
+
+	// can't keep the second exclusive name but make it nonexclusive and start a transaction
+	if trns, err := api0.StartTransaction(ctx, "exc2", time.Minute, false, false); errors.Cause(err) != pilosa.ErrTransactionExclusive {
+		t.Errorf("unexpected error: %v", err)
+	} else {
+		test.CompareTransactions(t, excTrns, trns)
 	}
 
 	// transaction is active on other nodes with remote=true
diff --git a/transaction.go b/transaction.go
index 3fa2e2a42..1e142ccb2 100644
--- a/transaction.go
+++ b/transaction.go
@@ -330,7 +330,7 @@ func (s *InMemTransactionStore) Put(trns *Transaction) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	s.tmap[trns.ID] = trns
+	s.tmap[trns.ID] = trns.Copy()
 	return nil
 }
 
@@ -339,7 +339,7 @@ func (s *InMemTransactionStore) Get(id string) (*Transaction, error) {
 	defer s.mu.RUnlock()
 
 	if trns, ok := s.tmap[id]; ok {
-		return trns, nil
+		return trns.Copy(), nil
 	}
 	return nil, ErrTransactionNotFound
 }
@@ -347,7 +347,7 @@ func (s *InMemTransactionStore) Get(id string) (*Transaction, error) {
 func (s *InMemTransactionStore) List() (map[string]*Transaction, error) {
 	cp := make(map[string]*Transaction)
 	for id, trns := range s.tmap {
-		cp[id] = trns
+		cp[id] = trns.Copy()
 	}
 	return cp, nil
 }
@@ -358,7 +358,7 @@ func (s *InMemTransactionStore) Remove(id string) (*Transaction, error) {
 
 	if trns, ok := s.tmap[id]; ok {
 		delete(s.tmap, id)
-		return trns, nil
+		return trns.Copy(), nil
 	}
 	return nil, ErrTransactionNotFound
 }
@@ -448,3 +448,14 @@ func (trns *Transaction) MarshalJSON() ([]byte, error) {
 		Deadline:  trns.Deadline.In(time.UTC).Format(time.RFC3339Nano),
 	})
 }
+
+func (trns *Transaction) Copy() *Transaction {
+	return &Transaction{
+		ID:        trns.ID,
+		Active:    trns.Active,
+		Exclusive: trns.Exclusive,
+		Timeout:   trns.Timeout,
+		Deadline:  trns.Deadline,
+		Stats:     trns.Stats,
+	}
+}