Skip to content

Commit c0d5e89

Browse files
committed
internal/lsp/fake: hold the mutex for all of SaveBufferWithoutActions
This addresses the race seen in https://storage.googleapis.com/go-build-log/5ba1c3f2/windows-386-2008_2bf3acc7.log. Paraphrased from Rob in chat: The race is triggered because of the additional asynchronous handling of file changes added to sync non-dirty files on disk changes. The buffer is marked as non-dirty, THEN the on-disk change is processed, sending a didChange, THEN the didSave is sent (https://golang.org/cl/267577). Also, don't send didChange for formatting if nothing has changed. Change-Id: Ibc6a55d35dee99c04dadf5470c2d68ca937f849b Reviewed-on: https://go-review.googlesource.com/c/tools/+/271578 Trust: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
1 parent be796f8 commit c0d5e89

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

internal/lsp/fake/editor.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,9 @@ func (e *Editor) SaveBuffer(ctx context.Context, path string) error {
397397

398398
func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) error {
399399
e.mu.Lock()
400+
defer e.mu.Unlock()
400401
buf, ok := e.buffers[path]
401402
if !ok {
402-
e.mu.Unlock()
403403
return fmt.Errorf(fmt.Sprintf("unknown buffer: %q", path))
404404
}
405405
content := buf.text()
@@ -408,7 +408,6 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro
408408
if ok {
409409
includeText = syncOptions.Save.IncludeText
410410
}
411-
e.mu.Unlock()
412411

413412
docID := e.textDocumentIdentifier(buf.path)
414413
if e.Server != nil {
@@ -423,10 +422,8 @@ func (e *Editor) SaveBufferWithoutActions(ctx context.Context, path string) erro
423422
return errors.Errorf("writing %q: %w", path, err)
424423
}
425424

426-
e.mu.Lock()
427425
buf.dirty = false
428426
e.buffers[path] = buf
429-
e.mu.Unlock()
430427

431428
if e.Server != nil {
432429
params := &protocol.DidSaveTextDocumentParams{
@@ -812,6 +809,9 @@ func (e *Editor) FormatBuffer(ctx context.Context, path string) error {
812809
return fmt.Errorf("before receipt of formatting edits, buffer version changed from %d to %d", version, versionAfter)
813810
}
814811
edits := convertEdits(resp)
812+
if len(edits) == 0 {
813+
return nil
814+
}
815815
return e.editBufferLocked(ctx, path, edits)
816816
}
817817

0 commit comments

Comments
 (0)