Fix races and remove unreachable code

josebalius · josebalius · commit 2406f3f09a0d · 2021-10-07T15:32:07.000-04:00
diff --git a/pkg/liveshare/session.go b/pkg/liveshare/session.go
@@ -114,15 +114,14 @@ func (s *Session) heartbeat(ctx context.Context, interval time.Duration) {
 		case <-ctx.Done():
 			return
 		case <-ticker.C:
-			s.logger.Println("Running session heartbeat")
+			s.logger.Println("Heartbeat tick")
 			reason := <-s.keepAliveReason
 			s.logger.Println("Keep alive reason: " + reason)
 			if err := s.notifyHostOfActivity(ctx, reason); err != nil {
 				s.logger.Printf("Failed to notify host of activity: %s\n", err)
 			}
 		}
 	}
-	s.logger.Println("Ending session heartbeat")
 }
 
 // notifyHostOfActivity notifies the Live Share host of client activity.
diff --git a/pkg/liveshare/session_test.go b/pkg/liveshare/session_test.go
@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -295,9 +296,14 @@ func TestNotifyHostOfActivity(t *testing.T) {
 }
 
 func TestSessionHeartbeat(t *testing.T) {
-	var requests int
+	var (
+		requestsMu sync.Mutex
+		requests   int
+	)
 	notifyHostOfActivity := func(rpcReq *jsonrpc2.Request) (interface{}, error) {
+		requestsMu.Lock()
 		requests++
+		requestsMu.Unlock()
 
 		var req []interface{}
 		if err := json.Unmarshal(*rpcReq.Params, &req); err != nil {
@@ -369,21 +375,28 @@ func TestSessionHeartbeat(t *testing.T) {
 }
 
 type mockLogger struct {
+	sync.Mutex
 	buf *bytes.Buffer
 }
 
 func newMockLogger() *mockLogger {
-	return &mockLogger{new(bytes.Buffer)}
+	return &mockLogger{buf: new(bytes.Buffer)}
 }
 
 func (m *mockLogger) Printf(format string, v ...interface{}) (int, error) {
+	m.Lock()
+	defer m.Unlock()
 	return m.buf.WriteString(fmt.Sprintf(format, v...))
 }
 
 func (m *mockLogger) Println(v ...interface{}) (int, error) {
+	m.Lock()
+	defer m.Unlock()
 	return m.buf.WriteString(fmt.Sprintln(v...))
 }
 
 func (m *mockLogger) String() string {
+	m.Lock()
+	defer m.Unlock()
 	return m.buf.String()
 }

Original file line number	Diff line number	Diff line change
`@@ -114,15 +114,14 @@ func (s *Session) heartbeat(ctx context.Context, interval time.Duration) {`
`114`	`114`	`case <-ctx.Done():`
`115`	`115`	`return`
`116`	`116`	`case <-ticker.C:`
`117`		`- s.logger.Println("Running session heartbeat")`
	`117`	`+ s.logger.Println("Heartbeat tick")`
`118`	`118`	`reason := <-s.keepAliveReason`
`119`	`119`	`s.logger.Println("Keep alive reason: " + reason)`
`120`	`120`	`if err := s.notifyHostOfActivity(ctx, reason); err != nil {`
`121`	`121`	`s.logger.Printf("Failed to notify host of activity: %s\n", err)`
`122`	`122`	`}`
`123`	`123`	`}`
`124`	`124`	`}`
`125`		`- s.logger.Println("Ending session heartbeat")`
`126`	`125`	`}`
`127`	`126`
`128`	`127`	`// notifyHostOfActivity notifies the Live Share host of client activity.`