Skip to content

Commit

Permalink
fixes 4 doria
Browse files Browse the repository at this point in the history
  • Loading branch information
mcy committed Sep 20, 2024
1 parent 49d3269 commit ec8e8e2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ issues:
# trip this off.
path: private/buf/bufcli/env.go
text: "G101:"
- linters:
- gosec
# G404 checks for use of the ordinary non-CPRNG.
path: private/buf/buflsp/progress.go
text: "G404:"
- linters:
- containedctx
# Type must implement an interface whose methods do not accept context. But this
Expand Down
38 changes: 8 additions & 30 deletions private/buf/buflsp/buflsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"context"
"fmt"
"sync/atomic"
"time"

"github.com/bufbuild/buf/private/buf/bufctl"
"github.com/bufbuild/buf/private/bufpkg/bufcheck"
Expand All @@ -38,10 +37,6 @@ import (
"go.uber.org/zap"
)

const (
handlerTimeout = 3 * time.Second
)

// Serve spawns a new LSP server, listening on the given stream.
//
// Returns a context for managing the server.
Expand Down Expand Up @@ -177,34 +172,17 @@ func (l *lsp) newHandler() jsonrpc2.Handler {
zap.ByteString("params", req.Params()),
)

// Each request is handled in a separate goroutine, and has a fixed timeout.
// This is to enforce responsiveness on the LSP side: if something is going to take
// a long time, it should be offloaded.
ctx, done := context.WithTimeout(ctx, handlerTimeout)
ctx = withRequestID(ctx)

go func() {
defer done()
replier := l.adaptReplier(reply, req)

// Verify that the server has been initialized if this isn't the initialization
// request.
if req.Method() != protocol.MethodInitialize && l.initParams.Load() == nil {
retErr = replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method",
protocol.MethodInitialize))
return
}

retErr = actual(ctx, replier, req)
}()

<-ctx.Done()
if ctx.Err() == context.DeadlineExceeded {
// Don't return this error; that will kill the whole server!
l.logger.Sugar().Errorf("timed out while handling %s; this is likely a bug", req.Method())
return nil
replier := l.adaptReplier(reply, req)

// Verify that the server has been initialized if this isn't the initialization
// request.
if req.Method() != protocol.MethodInitialize && l.initParams.Load() == nil {
return replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method", protocol.MethodInitialize))
}
return retErr

return actual(ctx, replier, req)
}
}

Expand Down
8 changes: 4 additions & 4 deletions private/buf/buflsp/mutex.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,6 @@ func (mu *mutex) Lock(ctx context.Context) (unlocker func()) {
panic("buflsp/mutex.go: non-reentrant lock locked twice by the same request")
}

// Block until we acquire the lock.
mu.lock.Lock()
mu.storeWho(id)

if mu.pool != nil {
mu.pool.lock.Lock()
defer mu.pool.lock.Unlock()
Expand All @@ -131,6 +127,10 @@ func (mu *mutex) Lock(ctx context.Context) (unlocker func()) {
mu.pool.held[id] = mu
}

// Ok, we're definitely not holding a lock, so we can block until we acquire the lock.
mu.lock.Lock()
mu.storeWho(id)

return unlocker
}

Expand Down

0 comments on commit ec8e8e2

Please sign in to comment.