From eafed880aec63923d946facaa1a0d47dba64224c Mon Sep 17 00:00:00 2001
From: Lautaro Emanuel <31224949+emlautarom1@users.noreply.github.com>
Date: Thu, 30 May 2024 10:36:34 -0300
Subject: [PATCH] Fix `SafeEthClient` concurrent close (#200)

* Add `TestConcurrentClose`

* Use `sync.Once` to prevent double closing

* Add `wg.Wait` to wait for all goroutines to finish before finishing the test
---
 core/safeclient/client.go      | 16 ++++++----------
 core/safeclient/client_test.go | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/core/safeclient/client.go b/core/safeclient/client.go
index a5f0b31f..8035294a 100644
--- a/core/safeclient/client.go
+++ b/core/safeclient/client.go
@@ -35,13 +35,13 @@ type SafeEthClient struct {
 	logger           logging.Logger
 	rpcUrl           string
 	closeC           chan struct{}
-	closed           bool
 	headerTimeout    time.Duration
 	logResubInterval time.Duration
 	blockChunkSize   uint64
 	blockMaxRange    uint64
 
 	createClient func(string, logging.Logger) (eth.Client, error)
+	onceClose    sync.Once
 }
 
 func NewSafeEthClient(rpcUrl string, logger logging.Logger, opts ...SafeEthClientOption) (*SafeEthClient, error) {
@@ -301,15 +301,11 @@ func (c *SafeEthClient) SubscribeFilterLogs(ctx context.Context, q ethereum.Filt
 }
 
 func (c *SafeEthClient) Close() {
-	if c.closed {
-		return
-	}
-
-	close(c.closeC)
-	c.wg.Wait()
-	c.logger.Info("SafeEthClient closed")
-
-	c.closed = true
+	c.onceClose.Do(func() {
+		close(c.closeC)
+		c.wg.Wait()
+		c.logger.Info("SafeEthClient closed")
+	})
 }
 
 func (c *SafeEthClient) SubscribeNewHead(ctx context.Context, ch chan<- *types.Header) (ethereum.Subscription, error) {
diff --git a/core/safeclient/client_test.go b/core/safeclient/client_test.go
index 9d1eb6d6..a4f97d4d 100644
--- a/core/safeclient/client_test.go
+++ b/core/safeclient/client_test.go
@@ -389,6 +389,24 @@ func NewMockSafeClientControllable(ctx context.Context, mockCtrl *gomock.Control
 	return client, err
 }
 
+func TestConcurrentClose(t *testing.T) {
+	logger, err := logging.NewZapLogger("development")
+	assert.NoError(t, err)
+
+	client, err := safeclient.NewSafeEthClient("", logger, safeclient.WithCustomCreateClient(func(string, logging.Logger) (eth.Client, error) { return nil, nil }))
+	assert.NoError(t, err)
+
+	var wg sync.WaitGroup
+	for i := 1; i <= 10; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			client.Close()
+		}()
+	}
+	wg.Wait()
+}
+
 func TestSubscribeNewHead(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()