From d5171129944de3cf2df348ae541cba5966fa4461 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 12 Jan 2024 11:50:48 -0500 Subject: [PATCH] gopls/internal/lsp/protocol: deliver log messages in order This fixes a predicted bug that was noticed while reading the code. Fixes golang/go#61216 Change-Id: I9614454fbd8538cb0b9eb1f56f11934cb88a7aed Reviewed-on: https://go-review.googlesource.com/c/tools/+/555635 Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI --- gopls/internal/lsp/protocol/context.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/gopls/internal/lsp/protocol/context.go b/gopls/internal/lsp/protocol/context.go index a9ef48d0f0b..5f3151cda97 100644 --- a/gopls/internal/lsp/protocol/context.go +++ b/gopls/internal/lsp/protocol/context.go @@ -7,6 +7,7 @@ package protocol import ( "bytes" "context" + "sync" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/event/core" @@ -38,8 +39,27 @@ func LogEvent(ctx context.Context, ev core.Event, lm label.Map, mt MessageType) if event.IsError(ev) { msg.Type = Error } - // TODO(adonovan): the goroutine here could cause log - // messages to be delivered out of order! Use a queue. - go client.LogMessage(xcontext.Detach(ctx), msg) + + // The background goroutine lives forever once started, + // and ensures log messages are sent in order (#61216). + startLogSenderOnce.Do(func() { + go func() { + for f := range logQueue { + f() + } + }() + }) + + // Add the log item to a queue, rather than sending a + // window/logMessage request to the client synchronously, + // which would slow down this thread. + ctx2 := xcontext.Detach(ctx) + logQueue <- func() { client.LogMessage(ctx2, msg) } + return ctx } + +var ( + startLogSenderOnce sync.Once + logQueue = make(chan func(), 100) // big enough for a large transient burst +)