From 14d3270d5b190ef73e27207517a8accea49ef613 Mon Sep 17 00:00:00 2001 From: Nodir Turakulov Date: Wed, 13 Apr 2022 14:29:22 -0700 Subject: [PATCH] Fix potential goroutine leak Sending to an unbuffered channel without a select and without a guarantee that something will receive from that channel, is a recipe for a gourutine leak. Make the channel buffered to avoid it. Also, with a buffered channel, no need to spin up a new goroutine to begin with. Just send right after channel creation. --- agent/acs/client/acs_client_test.go | 6 +++--- agent/acs/handler/acs_handler.go | 6 ++---- agent/tcs/handler/handler.go | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/agent/acs/client/acs_client_test.go b/agent/acs/client/acs_client_test.go index 6e56e84f55f..682c5608b20 100644 --- a/agent/acs/client/acs_client_test.go +++ b/agent/acs/client/acs_client_test.go @@ -347,9 +347,9 @@ func testCS(conn *mock_wsconn.MockWebsocketConn) wsclient.ClientServer { // TODO: replace with gomock func startMockAcsServer(t *testing.T, closeWS <-chan bool) (*httptest.Server, chan<- string, <-chan string, <-chan error, error) { - serverChan := make(chan string) - requestsChan := make(chan string) - errChan := make(chan error) + serverChan := make(chan string, 1) + requestsChan := make(chan string, 1) + errChan := make(chan error, 1) upgrader := websocket.Upgrader{ReadBufferSize: 1024, WriteBufferSize: 1024} handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/agent/acs/handler/acs_handler.go b/agent/acs/handler/acs_handler.go index b52feedba2f..f65ccfaba3a 100644 --- a/agent/acs/handler/acs_handler.go +++ b/agent/acs/handler/acs_handler.go @@ -196,12 +196,10 @@ func NewSession( func (acsSession *session) Start() error { // connectToACS channel is used to indicate the intent to connect to ACS // It's processed by the select loop to connect to ACS - connectToACS := make(chan struct{}) + connectToACS := make(chan struct{}, 1) // This is required to trigger the first connection to ACS. Subsequent // connections are triggered by the handleACSError() method - go func() { - connectToACS <- struct{}{} - }() + connectToACS <- struct{}{} for { select { case <-connectToACS: diff --git a/agent/tcs/handler/handler.go b/agent/tcs/handler/handler.go index b9ef6789431..70cf1ee14d3 100644 --- a/agent/tcs/handler/handler.go +++ b/agent/tcs/handler/handler.go @@ -143,7 +143,7 @@ func startSession( client.AddRequestHandler(ackPublishHealthMetricHandler(timer)) client.AddRequestHandler(ackPublishInstanceStatusHandler(timer)) client.SetAnyRequestHandler(anyMessageHandler(client)) - serveC := make(chan error) + serveC := make(chan error, 1) go func() { serveC <- client.Serve() }()