Skip to content

Commit feeb0ba

Browse files
findleyrgopherbot
findleyr
authored and
gopherbot
committed
gopls/internal/lsp/cmd: fix deadlock when opening a file
Rename (*connection).AddFile to openFile, which is more accurate, and fix a deadlock resulting from holding a Client lock while issuing a Server request. Fixes golang/go#56450 Change-Id: Ie6f34613e1e10e3274c3e6728b12f77e3a523b89 Reviewed-on: https://go-review.googlesource.com/c/tools/+/446856 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]> Auto-Submit: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 6e9dc86 commit feeb0ba

17 files changed

+37
-35
lines changed

‎gopls/internal/lsp/cmd/call_hierarchy.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (c *callHierarchy) Run(ctx context.Context, args ...string) error {
4747
defer conn.terminate(ctx)
4848

4949
from := span.Parse(args[0])
50-
file := conn.AddFile(ctx, from.URI())
50+
file := conn.openFile(ctx, from.URI())
5151
if file.err != nil {
5252
return file.err
5353
}
@@ -114,7 +114,7 @@ func (c *callHierarchy) Run(ctx context.Context, args ...string) error {
114114
// callItemPrintString returns a protocol.CallHierarchyItem object represented as a string.
115115
// item and call ranges (protocol.Range) are converted to user friendly spans (1-indexed).
116116
func callItemPrintString(ctx context.Context, conn *connection, item protocol.CallHierarchyItem, callsURI protocol.DocumentURI, calls []protocol.Range) (string, error) {
117-
itemFile := conn.AddFile(ctx, item.URI.SpanURI())
117+
itemFile := conn.openFile(ctx, item.URI.SpanURI())
118118
if itemFile.err != nil {
119119
return "", itemFile.err
120120
}
@@ -123,7 +123,7 @@ func callItemPrintString(ctx context.Context, conn *connection, item protocol.Ca
123123
return "", err
124124
}
125125

126-
callsFile := conn.AddFile(ctx, callsURI.SpanURI())
126+
callsFile := conn.openFile(ctx, callsURI.SpanURI())
127127
if callsURI != "" && callsFile.err != nil {
128128
return "", callsFile.err
129129
}

‎gopls/internal/lsp/cmd/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (c *check) Run(ctx context.Context, args ...string) error {
4848
for _, arg := range args {
4949
uri := span.URIFromPath(arg)
5050
uris = append(uris, uri)
51-
file := conn.AddFile(ctx, uri)
51+
file := conn.openFile(ctx, uri)
5252
if file.err != nil {
5353
return file.err
5454
}

‎gopls/internal/lsp/cmd/cmd.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ type cmdFile struct {
399399
uri span.URI
400400
mapper *protocol.ColumnMapper
401401
err error
402-
added bool
402+
open bool
403403
diagnostics []protocol.Diagnostic
404404
}
405405

@@ -558,22 +558,24 @@ func (c *cmdClient) getFile(ctx context.Context, uri span.URI) *cmdFile {
558558
return file
559559
}
560560

561-
func (c *connection) AddFile(ctx context.Context, uri span.URI) *cmdFile {
562-
c.Client.filesMu.Lock()
563-
defer c.Client.filesMu.Unlock()
561+
func (c *cmdClient) openFile(ctx context.Context, uri span.URI) *cmdFile {
562+
c.filesMu.Lock()
563+
defer c.filesMu.Unlock()
564564

565-
file := c.Client.getFile(ctx, uri)
566-
// This should never happen.
567-
if file == nil {
568-
return &cmdFile{
569-
uri: uri,
570-
err: fmt.Errorf("no file found for %s", uri),
571-
}
565+
file := c.getFile(ctx, uri)
566+
if file.err != nil || file.open {
567+
return file
572568
}
573-
if file.err != nil || file.added {
569+
file.open = true
570+
return file
571+
}
572+
573+
func (c *connection) openFile(ctx context.Context, uri span.URI) *cmdFile {
574+
file := c.Client.openFile(ctx, uri)
575+
if file.err != nil {
574576
return file
575577
}
576-
file.added = true
578+
577579
p := &protocol.DidOpenTextDocumentParams{
578580
TextDocument: protocol.TextDocumentItem{
579581
URI: protocol.URIFromSpanURI(uri),

‎gopls/internal/lsp/cmd/definition.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
8080
}
8181
defer conn.terminate(ctx)
8282
from := span.Parse(args[0])
83-
file := conn.AddFile(ctx, from.URI())
83+
file := conn.openFile(ctx, from.URI())
8484
if file.err != nil {
8585
return file.err
8686
}
@@ -113,7 +113,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error {
113113
if hover == nil {
114114
return fmt.Errorf("%v: not an identifier", from)
115115
}
116-
file = conn.AddFile(ctx, fileURI(locs[0].URI))
116+
file = conn.openFile(ctx, fileURI(locs[0].URI))
117117
if file.err != nil {
118118
return fmt.Errorf("%v: %v", from, file.err)
119119
}

‎gopls/internal/lsp/cmd/folding_range.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (r *foldingRanges) Run(ctx context.Context, args ...string) error {
4444
defer conn.terminate(ctx)
4545

4646
from := span.Parse(args[0])
47-
file := conn.AddFile(ctx, from.URI())
47+
file := conn.openFile(ctx, from.URI())
4848
if file.err != nil {
4949
return file.err
5050
}

‎gopls/internal/lsp/cmd/format.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (c *format) Run(ctx context.Context, args ...string) error {
5757
defer conn.terminate(ctx)
5858
for _, arg := range args {
5959
spn := span.Parse(arg)
60-
file := conn.AddFile(ctx, spn.URI())
60+
file := conn.openFile(ctx, spn.URI())
6161
if file.err != nil {
6262
return file.err
6363
}

‎gopls/internal/lsp/cmd/highlight.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (r *highlight) Run(ctx context.Context, args ...string) error {
4747
defer conn.terminate(ctx)
4848

4949
from := span.Parse(args[0])
50-
file := conn.AddFile(ctx, from.URI())
50+
file := conn.openFile(ctx, from.URI())
5151
if file.err != nil {
5252
return file.err
5353
}

‎gopls/internal/lsp/cmd/implementation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (i *implementation) Run(ctx context.Context, args ...string) error {
4747
defer conn.terminate(ctx)
4848

4949
from := span.Parse(args[0])
50-
file := conn.AddFile(ctx, from.URI())
50+
file := conn.openFile(ctx, from.URI())
5151
if file.err != nil {
5252
return file.err
5353
}
@@ -71,7 +71,7 @@ func (i *implementation) Run(ctx context.Context, args ...string) error {
7171

7272
var spans []string
7373
for _, impl := range implementations {
74-
f := conn.AddFile(ctx, fileURI(impl.URI))
74+
f := conn.openFile(ctx, fileURI(impl.URI))
7575
span, err := f.mapper.Span(impl)
7676
if err != nil {
7777
return err

‎gopls/internal/lsp/cmd/imports.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (t *imports) Run(ctx context.Context, args ...string) error {
5656

5757
from := span.Parse(args[0])
5858
uri := from.URI()
59-
file := conn.AddFile(ctx, uri)
59+
file := conn.openFile(ctx, uri)
6060
if file.err != nil {
6161
return file.err
6262
}

‎gopls/internal/lsp/cmd/links.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (l *links) Run(ctx context.Context, args ...string) error {
5353

5454
from := span.Parse(args[0])
5555
uri := from.URI()
56-
file := conn.AddFile(ctx, uri)
56+
file := conn.openFile(ctx, uri)
5757
if file.err != nil {
5858
return file.err
5959
}

‎gopls/internal/lsp/cmd/prepare_rename.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (r *prepareRename) Run(ctx context.Context, args ...string) error {
5151
defer conn.terminate(ctx)
5252

5353
from := span.Parse(args[0])
54-
file := conn.AddFile(ctx, from.URI())
54+
file := conn.openFile(ctx, from.URI())
5555
if file.err != nil {
5656
return file.err
5757
}

‎gopls/internal/lsp/cmd/references.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (r *references) Run(ctx context.Context, args ...string) error {
5151
defer conn.terminate(ctx)
5252

5353
from := span.Parse(args[0])
54-
file := conn.AddFile(ctx, from.URI())
54+
file := conn.openFile(ctx, from.URI())
5555
if file.err != nil {
5656
return file.err
5757
}
@@ -74,7 +74,7 @@ func (r *references) Run(ctx context.Context, args ...string) error {
7474
}
7575
var spans []string
7676
for _, l := range locations {
77-
f := conn.AddFile(ctx, fileURI(l.URI))
77+
f := conn.openFile(ctx, fileURI(l.URI))
7878
// convert location to span for user-friendly 1-indexed line
7979
// and column numbers
8080
span, err := f.mapper.Span(l)

‎gopls/internal/lsp/cmd/rename.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (r *rename) Run(ctx context.Context, args ...string) error {
6161
defer conn.terminate(ctx)
6262

6363
from := span.Parse(args[0])
64-
file := conn.AddFile(ctx, from.URI())
64+
file := conn.openFile(ctx, from.URI())
6565
if file.err != nil {
6666
return file.err
6767
}
@@ -92,7 +92,7 @@ func (r *rename) Run(ctx context.Context, args ...string) error {
9292

9393
for _, u := range orderedURIs {
9494
uri := span.URIFromURI(u)
95-
cmdFile := conn.AddFile(ctx, uri)
95+
cmdFile := conn.openFile(ctx, uri)
9696
filename := cmdFile.uri.Filename()
9797

9898
newContent, renameEdits, err := source.ApplyProtocolEdits(cmdFile.mapper, edits[uri])

‎gopls/internal/lsp/cmd/semantictokens.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (c *semtok) Run(ctx context.Context, args ...string) error {
8282
}
8383
defer conn.terminate(ctx)
8484
uri := span.URIFromPath(args[0])
85-
file := conn.AddFile(ctx, uri)
85+
file := conn.openFile(ctx, uri)
8686
if file.err != nil {
8787
return file.err
8888
}

‎gopls/internal/lsp/cmd/signature.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (r *signature) Run(ctx context.Context, args ...string) error {
4646
defer conn.terminate(ctx)
4747

4848
from := span.Parse(args[0])
49-
file := conn.AddFile(ctx, from.URI())
49+
file := conn.openFile(ctx, from.URI())
5050
if file.err != nil {
5151
return file.err
5252
}

‎gopls/internal/lsp/cmd/suggested_fix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (s *suggestedFix) Run(ctx context.Context, args ...string) error {
5656

5757
from := span.Parse(args[0])
5858
uri := from.URI()
59-
file := conn.AddFile(ctx, uri)
59+
file := conn.openFile(ctx, uri)
6060
if file.err != nil {
6161
return file.err
6262
}

‎gopls/internal/lsp/cmd/workspace_symbol.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (r *workspaceSymbol) Run(ctx context.Context, args ...string) error {
7373
return err
7474
}
7575
for _, s := range symbols {
76-
f := conn.AddFile(ctx, fileURI(s.Location.URI))
76+
f := conn.openFile(ctx, fileURI(s.Location.URI))
7777
span, err := f.mapper.Span(s.Location)
7878
if err != nil {
7979
return err

0 commit comments

Comments
 (0)