From 7e58f02eb84cc51c497aff6bb62a660782a9a696 Mon Sep 17 00:00:00 2001
From: Sam Kleinman <garen@tychoish.com>
Date: Fri, 19 Nov 2021 11:49:51 -0500
Subject: [PATCH] service: remove quit method (#7293)

---
 internal/proxy/multi_app_conn.go      | 58 ++++++++++++++++++---------
 internal/proxy/multi_app_conn_test.go | 17 +++-----
 libs/service/service.go               |  3 --
 3 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/internal/proxy/multi_app_conn.go b/internal/proxy/multi_app_conn.go
index 5e5920f24..6196992ff 100644
--- a/internal/proxy/multi_app_conn.go
+++ b/internal/proxy/multi_app_conn.go
@@ -128,7 +128,7 @@ func (app *multiAppConn) OnStart(ctx context.Context) error {
 	app.consensusConn = NewAppConnConsensus(c, app.metrics)
 
 	// Kill Tendermint if the ABCI application crashes.
-	go app.killTMOnClientError()
+	app.startWatchersForClientErrorToKillTendermint(ctx)
 
 	return nil
 }
@@ -137,7 +137,12 @@ func (app *multiAppConn) OnStop() {
 	app.stopAllClients()
 }
 
-func (app *multiAppConn) killTMOnClientError() {
+func (app *multiAppConn) startWatchersForClientErrorToKillTendermint(ctx context.Context) {
+	// this function starts a number of threads (per abci client)
+	// that will SIGTERM's our own PID if any of the ABCI clients
+	// exit/return early. If the context is canceled then these
+	// functions will not kill tendermint.
+
 	killFn := func(conn string, err error, logger log.Logger) {
 		logger.Error(
 			fmt.Sprintf("%s connection terminated. Did the application crash? Please restart tendermint", conn),
@@ -147,23 +152,38 @@ func (app *multiAppConn) killTMOnClientError() {
 		}
 	}
 
-	select {
-	case <-app.consensusConnClient.Quit():
-		if err := app.consensusConnClient.Error(); err != nil {
-			killFn(connConsensus, err, app.Logger)
-		}
-	case <-app.mempoolConnClient.Quit():
-		if err := app.mempoolConnClient.Error(); err != nil {
-			killFn(connMempool, err, app.Logger)
-		}
-	case <-app.queryConnClient.Quit():
-		if err := app.queryConnClient.Error(); err != nil {
-			killFn(connQuery, err, app.Logger)
-		}
-	case <-app.snapshotConnClient.Quit():
-		if err := app.snapshotConnClient.Error(); err != nil {
-			killFn(connSnapshot, err, app.Logger)
-		}
+	type op struct {
+		connClient stoppableClient
+		name       string
+	}
+
+	for _, client := range []op{
+		{
+			connClient: app.consensusConnClient,
+			name:       connConsensus,
+		},
+		{
+			connClient: app.mempoolConnClient,
+			name:       connMempool,
+		},
+		{
+			connClient: app.queryConnClient,
+			name:       connQuery,
+		},
+		{
+			connClient: app.snapshotConnClient,
+			name:       connSnapshot,
+		},
+	} {
+		go func(name string, client stoppableClient) {
+			client.Wait()
+			if ctx.Err() != nil {
+				return
+			}
+			if err := client.Error(); err != nil {
+				killFn(name, err, app.Logger)
+			}
+		}(client.name, client.connClient)
 	}
 }
 
diff --git a/internal/proxy/multi_app_conn_test.go b/internal/proxy/multi_app_conn_test.go
index 9ad39cb3b..98ea0ca53 100644
--- a/internal/proxy/multi_app_conn_test.go
+++ b/internal/proxy/multi_app_conn_test.go
@@ -26,14 +26,13 @@ type noopStoppableClientImpl struct {
 func (c *noopStoppableClientImpl) Stop() error { c.count++; return nil }
 
 func TestAppConns_Start_Stop(t *testing.T) {
-	quitCh := make(<-chan struct{})
-
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
 	clientMock := &abcimocks.Client{}
 	clientMock.On("Start", mock.Anything).Return(nil).Times(4)
-	clientMock.On("Quit").Return(quitCh).Times(4)
+	clientMock.On("Error").Return(nil)
+	clientMock.On("Wait").Return(nil).Times(4)
 	cl := &noopStoppableClientImpl{Client: clientMock}
 
 	creatorCallCount := 0
@@ -65,22 +64,19 @@ func TestAppConns_Failure(t *testing.T) {
 	go func() {
 		for range c {
 			close(ok)
+			return
 		}
 	}()
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	quitCh := make(chan struct{})
-	var recvQuitCh <-chan struct{} // nolint:gosimple
-	recvQuitCh = quitCh
-
 	clientMock := &abcimocks.Client{}
 	clientMock.On("SetLogger", mock.Anything).Return()
 	clientMock.On("Start", mock.Anything).Return(nil)
 
-	clientMock.On("Quit").Return(recvQuitCh)
-	clientMock.On("Error").Return(errors.New("EOF")).Once()
+	clientMock.On("Wait").Return(nil)
+	clientMock.On("Error").Return(errors.New("EOF"))
 	cl := &noopStoppableClientImpl{Client: clientMock}
 
 	creator := func(log.Logger) (abciclient.Client, error) {
@@ -93,9 +89,6 @@ func TestAppConns_Failure(t *testing.T) {
 	require.NoError(t, err)
 	t.Cleanup(func() { cancel(); appConns.Wait() })
 
-	// simulate failure
-	close(quitCh)
-
 	select {
 	case <-ok:
 		t.Log("SIGTERM successfully received")
diff --git a/libs/service/service.go b/libs/service/service.go
index 0fc26dbdb..f2b440e94 100644
--- a/libs/service/service.go
+++ b/libs/service/service.go
@@ -30,9 +30,6 @@ type Service interface {
 	// Return true if the service is running
 	IsRunning() bool
 
-	// Quit returns a channel, which is closed once service is stopped.
-	Quit() <-chan struct{}
-
 	// String representation of the service
 	String() string