mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: don't use overlays from the session in the snapshot 

Hold the session's overlay mutex the whole time we compute new overlays,
and then pass these overlays directly into clone. This avoids us calling
s.session.GetFile, which can return overlays that the snapshot doesn't
yet "know" about.

Change-Id: I1a10c78e26f8fec64550bfe0a97b5975ea8f976b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218321
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-02-06 16:20:50 -05:00
parent b753a1ba74
commit 3d51b05cfb
8 changed files with 158 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

118
internal/lsp/cache/overlay.go vendored
View File

@ -1,118 +0,0 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package cache
import (
"context"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
errors "golang.org/x/xerrors"
)
type overlay struct {
session *session
uri span.URI
text []byte
hash string
version float64
kind source.FileKind
// saved is true if a file has been saved on disk,
// and therefore does not need to be part of the overlay sent to go/packages.
saved bool
}
func (o *overlay) FileSystem() source.FileSystem {
return o.session
}
func (o *overlay) Identity() source.FileIdentity {
return source.FileIdentity{
URI: o.uri,
Identifier: o.hash,
Version: o.version,
Kind: o.kind,
}
}
func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
return o.text, o.hash, nil
}
func (s *session) updateOverlay(ctx context.Context, c source.FileModification) error {
// Make sure that the file was not changed on disk.
if c.OnDisk {
return errors.Errorf("updateOverlay called for an on-disk change: %s", c.URI)
}
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
o, ok := s.overlays[c.URI]
// Determine the file kind on open, otherwise, assume it has been cached.
var kind source.FileKind
switch c.Action {
case source.Open:
kind = source.DetectLanguage(c.LanguageID, c.URI.Filename())
default:
if !ok {
return errors.Errorf("updateOverlay: modifying unopened overlay %v", c.URI)
}
kind = o.kind
}
if kind == source.UnknownKind {
return errors.Errorf("updateOverlay: unknown file kind for %s", c.URI)
}
// Closing a file just deletes its overlay.
if c.Action == source.Close {
delete(s.overlays, c.URI)
return nil
}
// If the file is on disk, check if its content is the same as the overlay.
text := c.Text
if text == nil {
text = o.text
}
hash := hashContents(text)
var sameContentOnDisk bool
switch c.Action {
case source.Open:
_, h, err := s.cache.GetFile(c.URI).Read(ctx)
sameContentOnDisk = (err == nil && h == hash)
case source.Save:
// Make sure the version and content (if present) is the same.
if o.version != c.Version {
return errors.Errorf("updateOverlay: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
}
if c.Text != nil && o.hash != hash {
return errors.Errorf("updateOverlay: overlay %s changed on save", c.URI)
}
sameContentOnDisk = true
}
s.overlays[c.URI] = &overlay{
session: s,
uri: c.URI,
version: c.Version,
text: text,
kind: kind,
hash: hash,
saved: sameContentOnDisk,
}
return nil
}
func (s *session) readOverlay(uri span.URI) *overlay {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
// We might have the content saved in an overlay.
if overlay, ok := s.overlays[uri]; ok {
return overlay
}
return nil
}

139
internal/lsp/cache/session.go vendored
View File

@ -33,6 +33,35 @@ type session struct {
overlays map[span.URI]*overlay
}
type overlay struct {
session *session
uri span.URI
text []byte
hash string
version float64
kind source.FileKind
// saved is true if a file has been saved on disk,
// and therefore does not need to be part of the overlay sent to go/packages.
saved bool
}
func (o *overlay) FileSystem() source.FileSystem {
return o.session
}
func (o *overlay) Identity() source.FileIdentity {
return source.FileIdentity{
URI: o.uri,
Identifier: o.hash,
Version: o.version,
Kind: o.kind,
}
}
func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
return o.text, o.hash, nil
}
func (s *session) Options() source.Options {
return s.options
}
@ -249,14 +278,17 @@ func (s *session) dropView(ctx context.Context, v *view) (int, error) {
}
func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, error) {
views := make(map[*view][]span.URI)
views := make(map[*view]map[span.URI]source.FileHandle)
overlays, err := s.updateOverlays(ctx, changes)
if err != nil {
return nil, err
}
for _, c := range changes {
// Only update overlays for in-editor changes.
if !c.OnDisk {
if err := s.updateOverlay(ctx, c); err != nil {
return nil, err
}
// Do nothing if the file is open in the editor and we receive
// an on-disk action. The editor is the source of truth.
if s.isOpen(c.URI) && c.OnDisk {
continue
}
for _, view := range s.viewsOf(c.URI) {
if view.Ignore(c.URI) {
@ -276,7 +308,14 @@ func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModif
if _, err := view.getFile(c.URI); err != nil {
return nil, err
}
views[view] = append(views[view], c.URI)
if _, ok := views[view]; !ok {
views[view] = make(map[span.URI]source.FileHandle)
}
if o, ok := overlays[c.URI]; ok {
views[view][c.URI] = o
} else {
views[view][c.URI] = s.cache.GetFile(c.URI)
}
}
}
var snapshots []source.Snapshot
@ -286,7 +325,7 @@ func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModif
return snapshots, nil
}
func (s *session) IsOpen(uri span.URI) bool {
func (s *session) isOpen(uri span.URI) bool {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
@ -294,6 +333,80 @@ func (s *session) IsOpen(uri span.URI) bool {
return open
}
func (s *session) updateOverlays(ctx context.Context, changes []source.FileModification) (map[span.URI]*overlay, error) {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
for _, c := range changes {
// Don't update overlays for on-disk changes.
if c.OnDisk {
continue
}
o, ok := s.overlays[c.URI]
// Determine the file kind on open, otherwise, assume it has been cached.
var kind source.FileKind
switch c.Action {
case source.Open:
kind = source.DetectLanguage(c.LanguageID, c.URI.Filename())
default:
if !ok {
return nil, errors.Errorf("updateOverlays: modifying unopened overlay %v", c.URI)
}
kind = o.kind
}
if kind == source.UnknownKind {
return nil, errors.Errorf("updateOverlays: unknown file kind for %s", c.URI)
}
// Closing a file just deletes its overlay.
if c.Action == source.Close {
delete(s.overlays, c.URI)
continue
}
// If the file is on disk, check if its content is the same as the overlay.
text := c.Text
if text == nil {
text = o.text
}
hash := hashContents(text)
var sameContentOnDisk bool
switch c.Action {
case source.Open:
_, h, err := s.cache.GetFile(c.URI).Read(ctx)
sameContentOnDisk = (err == nil && h == hash)
case source.Save:
// Make sure the version and content (if present) is the same.
if o.version != c.Version {
return nil, errors.Errorf("updateOverlays: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
}
if c.Text != nil && o.hash != hash {
return nil, errors.Errorf("updateOverlays: overlay %s changed on save", c.URI)
}
sameContentOnDisk = true
}
o = &overlay{
session: s,
uri: c.URI,
version: c.Version,
text: text,
kind: kind,
hash: hash,
saved: sameContentOnDisk,
}
s.overlays[c.URI] = o
}
// Get the overlays for each change while the session's overlay map is locked.
overlays := make(map[span.URI]*overlay)
for _, c := range changes {
overlays[c.URI] = s.overlays[c.URI]
}
return overlays, nil
}
func (s *session) GetFile(uri span.URI) source.FileHandle {
if overlay := s.readOverlay(uri); overlay != nil {
return overlay
@ -301,3 +414,13 @@ func (s *session) GetFile(uri span.URI) source.FileHandle {
// Fall back to the cache-level file system.
return s.cache.GetFile(uri)
}
func (s *session) readOverlay(uri span.URI) *overlay {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
if overlay, ok := s.overlays[uri]; ok {
return overlay
}
return nil
}

21
internal/lsp/cache/snapshot.go vendored
View File

@ -108,6 +108,9 @@ func (s *snapshot) Config(ctx context.Context) *packages.Config {
}
func (s *snapshot) buildOverlay() map[string][]byte {
s.mu.Lock()
defer s.mu.Unlock()
overlays := make(map[string][]byte)
for uri, fh := range s.files {
overlay, ok := fh.(*overlay)
@ -569,15 +572,24 @@ func (s *snapshot) GetFile(uri span.URI) (source.FileHandle, error) {
if err != nil {
return nil, err
}
s.mu.Lock()
defer s.mu.Unlock()
if _, ok := s.files[f.URI()]; !ok {
s.files[f.URI()] = s.view.session.GetFile(f.URI())
s.files[f.URI()] = s.view.session.cache.GetFile(uri)
}
return s.files[f.URI()], nil
}
func (s *snapshot) IsOpen(uri span.URI) bool {
s.mu.Lock()
defer s.mu.Unlock()
_, open := s.files[uri].(*overlay)
return open
}
func (s *snapshot) awaitLoaded(ctx context.Context) error {
// Do not return results until the snapshot's view has been initialized.
s.view.awaitInitialized(ctx)
@ -685,7 +697,7 @@ func contains(views []*view, view *view) bool {
return false
}
func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot {
func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.FileHandle) *snapshot {
s.mu.Lock()
defer s.mu.Unlock()
@ -716,7 +728,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot
// If an ID's value is true, invalidate its metadata too.
transitiveIDs := make(map[packageID]bool)
for _, withoutURI := range withoutURIs {
for withoutURI, currentFH := range withoutURIs {
directIDs := map[packageID]struct{}{}
// Collect all of the package IDs that correspond to the given file.
@ -724,8 +736,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot
for _, id := range s.ids[withoutURI] {
directIDs[id] = struct{}{}
}
// Get the current and original FileHandles for this URI.
currentFH := s.view.session.GetFile(withoutURI)
// The original FileHandle for this URI is cached on the snapshot.
originalFH := s.files[withoutURI]
// Check if the file's package name or imports have changed,

7
internal/lsp/cache/view.go vendored
View File

@ -239,8 +239,9 @@ func (v *view) buildBuiltinPackage(ctx context.Context, goFiles []string) error
uri := span.FileURI(goFiles[0])
v.addIgnoredFile(uri) // to avoid showing diagnostics for builtin.go
// Get the FileHandle through the session to avoid adding it to the snapshot.
pgh := v.session.cache.ParseGoHandle(v.session.GetFile(uri), source.ParseFull)
// Get the FileHandle through the cache to avoid adding it to the snapshot
// and to get the file content from disk.
pgh := v.session.cache.ParseGoHandle(v.session.cache.GetFile(uri), source.ParseFull)
fset := v.session.cache.fset
h := v.session.cache.store.Bind(pgh.File().Identity(), func(ctx context.Context) interface{} {
data := &builtinPackageData{}
@ -540,7 +541,7 @@ func (v *view) awaitInitialized(ctx context.Context) {
// invalidateContent invalidates the content of a Go file,
// including any position and type information that depends on it.
// It returns true if we were already tracking the given file, false otherwise.
func (v *view) invalidateContent(ctx context.Context, uris []span.URI) source.Snapshot {
func (v *view) invalidateContent(ctx context.Context, uris map[span.URI]source.FileHandle) source.Snapshot {
// Detach the context so that content invalidation cannot be canceled.
ctx = xcontext.Detach(ctx)

2
internal/lsp/diagnostics.go
View File

@ -88,7 +88,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
// Only run analyses for packages with open files.
withAnalyses := alwaysAnalyze
for _, fh := range ph.CompiledGoFiles() {
if s.session.IsOpen(fh.File().Identity().URI) {
if snapshot.IsOpen(fh.File().Identity().URI) {
withAnalyses = true
}
}

2
internal/lsp/source/diagnostics.go
View File

@ -105,7 +105,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi
// If no file is associated with the error, pick an open file from the package.
if e.URI.Filename() == "" {
for _, ph := range pkg.CompiledGoFiles() {
if snapshot.View().Session().IsOpen(ph.File().Identity().URI) {
if snapshot.IsOpen(ph.File().Identity().URI) {
e.URI = ph.File().Identity().URI
}
}

8
internal/lsp/source/view.go
View File

@ -33,6 +33,10 @@ type Snapshot interface {
// if it is not already part of the view.
GetFile(uri span.URI) (FileHandle, error)
// IsOpen returns whether the editor currently has a file open,
// and if its contents are saved on disk or not.
IsOpen(uri span.URI) bool
// Analyze runs the analyses for the given package at this snapshot.
Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error)
@ -164,10 +168,6 @@ type Session interface {
// content from the underlying cache if no overlay is present.
FileSystem
// IsOpen returns whether the editor currently has a file open,
// and if its contents are saved on disk or not.
IsOpen(uri span.URI) bool
// DidModifyFile reports a file modification to the session.
// It returns the resulting snapshots, a guaranteed one per view.
DidModifyFiles(ctx context.Context, changes []FileModification) ([]Snapshot, error)

9
internal/lsp/text_synchronization.go
View File

@ -65,15 +65,8 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
var modifications []source.FileModification
for _, change := range params.Changes {
uri := span.NewURI(change.URI)
// Do nothing if the file is open in the editor.
// The editor is the source of truth.
if s.session.IsOpen(uri) {
continue
}
modifications = append(modifications, source.FileModification{
URI: uri,
URI: span.NewURI(change.URI),
Action: changeTypeToFileAction(change.Type),
OnDisk: true,
})