internal/lsp/regtest: allow sharing memoized results across regtests

Each regtest does a significant amount of extra work re-doing things
like parsing and type-checking the runtime package. We can share this
work across regtests by using a shared cache, significantly speeding
them up at the cost of potentially hiding bugs related to timing.

Sharing this work still retains most of the benefit of the regtests, so
implement this in the default mode (formerly called "singleton" and now
renamed to "default"). In a subsequent CL, modes will be cleaned up so
that "default" is the only mode that runs with -short.

Making this change actually revealed a caching bug: our cached package
stores error messages extracted from go/packages errors, but does not
include these errors in the cache key. Fix this by hashing all metadata
errors into the package cache key.

Updates golang/go#39384

Change-Id: I37ab9604149d34c9a79fc02b0e1bc23fcb17c454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417587
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This commit is contained in:
Robert Findley 2022-07-15 14:51:24 -04:00
parent 8ccb25c9a3
commit f157068c1b
22 changed files with 181 additions and 79 deletions

View File

@ -33,7 +33,7 @@ func benchmarkOptions(dir string) []RunOption {
// Skip logs as they buffer up memory unnaturally.
SkipLogs(),
// The Debug server only makes sense if running in singleton mode.
Modes(Singleton),
Modes(Default),
// Remove the default timeout. Individual tests should control their
// own graceful termination.
NoDefaultTimeout(),

View File

@ -20,7 +20,7 @@ func TestBugNotification(t *testing.T) {
// Verify that a properly configured session gets notified of a bug on the
// server.
WithOptions(
Modes(Singleton), // must be in-process to receive the bug report below
Modes(Default), // must be in-process to receive the bug report below
Settings{"showBugReports": true},
).Run(t, "", func(t *testing.T, env *Env) {
const desc = "got a bug"

View File

@ -298,7 +298,7 @@ func Hello() {
t.Run("without workspace module", func(t *testing.T) {
WithOptions(
Modes(Singleton),
Modes(Default),
).Run(t, noMod, func(t *testing.T, env *Env) {
env.Await(
env.DiagnosticAtRegexp("main.go", `"mod.com/bob"`),
@ -1678,7 +1678,7 @@ import (
WithOptions(
InGOPATH(),
EnvVars{"GO111MODULE": "off"},
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.Await(
env.DiagnosticAtRegexpWithMessage("main.go", `"nosuchpkg"`, `cannot find package "nosuchpkg" in any of`),
@ -1705,7 +1705,7 @@ package b
for _, go111module := range []string{"on", "auto"} {
t.Run("GO111MODULE="+go111module, func(t *testing.T) {
WithOptions(
Modes(Singleton),
Modes(Default),
EnvVars{"GO111MODULE": go111module},
).Run(t, modules, func(t *testing.T, env *Env) {
env.OpenFile("a/a.go")
@ -1722,7 +1722,7 @@ package b
// Expect no warning if GO111MODULE=auto in a directory in GOPATH.
t.Run("GOPATH_GO111MODULE_auto", func(t *testing.T) {
WithOptions(
Modes(Singleton),
Modes(Default),
EnvVars{"GO111MODULE": "auto"},
InGOPATH(),
).Run(t, modules, func(t *testing.T, env *Env) {
@ -1784,7 +1784,7 @@ func helloHelper() {}
`
WithOptions(
ProxyFiles(proxy),
Modes(Singleton),
Modes(Default),
).Run(t, nested, func(t *testing.T, env *Env) {
// Expect a diagnostic in a nested module.
env.OpenFile("nested/hello/hello.go")
@ -1996,7 +1996,7 @@ func Hello() {}
`
WithOptions(
Settings{"experimentalUseInvalidMetadata": true},
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.RegexpReplace("go.mod", "module mod.com", "modul mod.com") // break the go.mod file
@ -2052,7 +2052,7 @@ func _() {}
Settings{"experimentalUseInvalidMetadata": true},
// ExperimentalWorkspaceModule has a different failure mode for this
// case.
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.Await(
OnceMet(

View File

@ -25,7 +25,7 @@ func main() {}
`
WithOptions(
Modes(Singleton),
Modes(Default),
Settings{"allExperiments": true},
).Run(t, src, func(t *testing.T, env *Env) {
params := &protocol.SemanticTokensParams{}

View File

@ -49,7 +49,7 @@ func _() {
}
`
WithOptions(
Modes(Singleton),
Modes(Default),
ProxyFiles(basicProxy),
).Run(t, pkgThatUsesVendoring, func(t *testing.T, env *Env) {
env.OpenFile("a/a1.go")

View File

@ -742,7 +742,7 @@ func main() {
WithOptions(
EnvVars{"GOFLAGS": "-mod=readonly"},
ProxyFiles(proxy),
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
original := env.ReadWorkspaceFile("go.mod")
@ -922,7 +922,7 @@ func hello() {}
// TODO(rFindley) this doesn't work in multi-module workspace mode, because
// it keeps around the last parsing modfile. Update this test to also
// exercise the workspace module.
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.Await(env.DoneWithOpen())
@ -1090,7 +1090,7 @@ func main() {
`
WithOptions(
ProxyFiles(workspaceProxy),
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
params := &protocol.PublishDiagnosticsParams{}
@ -1159,7 +1159,7 @@ func main() {
`
WithOptions(
ProxyFiles(proxy),
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
d := &protocol.PublishDiagnosticsParams{}

View File

@ -1205,7 +1205,7 @@ package main
`
WithOptions(
EnvVars{"GOPATH": filepath.FromSlash("$SANDBOX_WORKDIR/gopath")},
Modes(Singleton),
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
env.Await(
// Confirm that the build configuration is seen as valid,
@ -1236,7 +1236,7 @@ package main
func main() {}
`
WithOptions(
Modes(Singleton),
Modes(Default),
).Run(t, nomod, func(t *testing.T, env *Env) {
env.OpenFile("a/main.go")
env.OpenFile("b/main.go")

View File

@ -293,7 +293,7 @@ func runTest(t *testing.T, workspaceData, proxyData string, test func(context.Co
t.Fatal(err)
}
cache := cache.New(nil)
cache := cache.New(nil, nil, nil)
session := cache.NewSession(ctx)
options := source.DefaultOptions().Clone()
tests.DefaultOptions(options)

View File

@ -28,23 +28,46 @@ import (
"golang.org/x/tools/internal/span"
)
func New(options func(*source.Options)) *Cache {
// New Creates a new cache for gopls operation results, using the given file
// set, shared store, and session options.
//
// All of the fset, store and options may be nil, but if store is non-nil so
// must be fset (and they must always be used together), otherwise it may be
// possible to get cached data referencing token.Pos values not mapped by the
// FileSet.
func New(fset *token.FileSet, store *memoize.Store, options func(*source.Options)) *Cache {
index := atomic.AddInt64(&cacheIndex, 1)
if store != nil && fset == nil {
panic("non-nil store with nil fset")
}
if fset == nil {
fset = token.NewFileSet()
}
if store == nil {
store = &memoize.Store{}
}
c := &Cache{
id: strconv.FormatInt(index, 10),
fset: token.NewFileSet(),
fset: fset,
options: options,
store: store,
fileContent: map[span.URI]*fileHandle{},
}
return c
}
type Cache struct {
id string
fset *token.FileSet
id string
fset *token.FileSet
// TODO(rfindley): it doesn't make sense that cache accepts LSP options, just
// so that it can create a session: the cache does not (and should not)
// depend on options. Invert this relationship to remove options from Cache.
options func(*source.Options)
store memoize.Store
store *memoize.Store
fileMu sync.Mutex
fileContent map[span.URI]*fileHandle

View File

@ -249,6 +249,16 @@ func computePackageKey(id PackageID, files []source.FileHandle, m *KnownMetadata
for _, file := range files {
b.WriteString(file.FileIdentity().String())
}
// Metadata errors are interpreted and memoized on the computed package, so
// we must hash them into the key here.
//
// TODO(rfindley): handle metadata diagnostics independently from
// type-checking diagnostics.
for _, err := range m.Errors {
b.WriteString(err.Msg)
b.WriteString(err.Pos)
b.WriteRune(rune(err.Kind))
}
return packageHandleKey(source.HashOf(b.Bytes()))
}

View File

@ -231,7 +231,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
backgroundCtx: backgroundCtx,
cancel: cancel,
initializeOnce: &sync.Once{},
store: &s.cache.store,
store: s.cache.store,
packages: persistent.NewMap(packageKeyLessInterface),
meta: &metadataGraph{},
files: newFilesMap(),

View File

@ -43,7 +43,7 @@ func TestCapabilities(t *testing.T) {
params.Capabilities.Workspace.Configuration = true
// Send an initialize request to the server.
c.Server = lsp.NewServer(cache.New(app.options).NewSession(ctx), c.Client)
c.Server = lsp.NewServer(cache.New(nil, nil, app.options).NewSession(ctx), c.Client)
result, err := c.Server.Initialize(ctx, params)
if err != nil {
t.Fatal(err)

View File

@ -286,7 +286,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) {
switch {
case app.Remote == "":
connection := newConnection(app)
connection.Server = lsp.NewServer(cache.New(app.options).NewSession(ctx), connection.Client)
connection.Server = lsp.NewServer(cache.New(nil, nil, app.options).NewSession(ctx), connection.Client)
ctx = protocol.WithClient(ctx, connection.Client)
return connection, connection.initialize(ctx, app.options)
case strings.HasPrefix(app.Remote, "internal@"):

View File

@ -101,7 +101,7 @@ func (s *Serve) Run(ctx context.Context, args ...string) error {
return fmt.Errorf("creating forwarder: %w", err)
}
} else {
ss = lsprpc.NewStreamServer(cache.New(s.app.options), isDaemon)
ss = lsprpc.NewStreamServer(cache.New(nil, nil, s.app.options), isDaemon)
}
var network, addr string

View File

@ -50,7 +50,7 @@ func TestCommandLine(t *testing.T, testdata string, options func(*source.Options
func NewTestServer(ctx context.Context, options func(*source.Options)) *servertest.TCPServer {
ctx = debug.WithInstance(ctx, "", "")
cache := cache.New(options)
cache := cache.New(nil, nil, options)
ss := lsprpc.NewStreamServer(cache, false)
return servertest.NewTCPServer(ctx, ss, nil)
}

View File

@ -49,7 +49,7 @@ type runner struct {
func testLSP(t *testing.T, datum *tests.Data) {
ctx := tests.Context(t)
cache := cache.New(nil)
cache := cache.New(nil, nil, nil)
session := cache.NewSession(ctx)
options := source.DefaultOptions().Clone()
tests.DefaultOptions(options)

View File

@ -58,7 +58,7 @@ func TestClientLogging(t *testing.T) {
client := FakeClient{Logs: make(chan string, 10)}
ctx = debug.WithInstance(ctx, "", "")
ss := NewStreamServer(cache.New(nil), false)
ss := NewStreamServer(cache.New(nil, nil, nil), false)
ss.serverForTest = server
ts := servertest.NewPipeServer(ss, nil)
defer checkClose(t, ts.Close)
@ -121,7 +121,7 @@ func checkClose(t *testing.T, closer func() error) {
func setupForwarding(ctx context.Context, t *testing.T, s protocol.Server) (direct, forwarded servertest.Connector, cleanup func()) {
t.Helper()
serveCtx := debug.WithInstance(ctx, "", "")
ss := NewStreamServer(cache.New(nil), false)
ss := NewStreamServer(cache.New(nil, nil, nil), false)
ss.serverForTest = s
tsDirect := servertest.NewTCPServer(serveCtx, ss, nil)
@ -216,7 +216,7 @@ func TestDebugInfoLifecycle(t *testing.T) {
clientCtx := debug.WithInstance(baseCtx, "", "")
serverCtx := debug.WithInstance(baseCtx, "", "")
cache := cache.New(nil)
cache := cache.New(nil, nil, nil)
ss := NewStreamServer(cache, false)
tsBackend := servertest.NewTCPServer(serverCtx, ss, nil)

View File

@ -26,7 +26,7 @@ func TestModfileRemainsUnchanged(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
ctx := tests.Context(t)
cache := cache.New(nil)
cache := cache.New(nil, nil, nil)
session := cache.NewSession(ctx)
options := source.DefaultOptions().Clone()
tests.DefaultOptions(options)

View File

@ -8,6 +8,7 @@ import (
"context"
"flag"
"fmt"
"go/token"
"io/ioutil"
"os"
"runtime"
@ -16,6 +17,7 @@ import (
"golang.org/x/tools/internal/lsp/cmd"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/memoize"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/internal/tool"
)
@ -87,9 +89,11 @@ var slowGOOS = map[string]bool{
}
func DefaultModes() Mode {
normal := Singleton | Experimental
// TODO(rfindley): these modes should *not* depend on GOOS. Depending on
// testing.Short() should be sufficient.
normal := Default | Experimental
if slowGOOS[runtime.GOOS] && testing.Short() {
normal = Singleton
normal = Default
}
if *runSubprocessTests {
return normal | SeparateProcess
@ -116,6 +120,8 @@ func Main(m *testing.M, hook func(*source.Options)) {
PrintGoroutinesOnFailure: *printGoroutinesOnFailure,
SkipCleanup: *skipCleanup,
OptionsHook: hook,
fset: token.NewFileSet(),
store: memoize.NewStore(memoize.NeverEvict),
}
if *runSubprocessTests {
goplsPath := *goplsBinaryPath
@ -126,13 +132,13 @@ func Main(m *testing.M, hook func(*source.Options)) {
panic(fmt.Sprintf("finding test binary path: %v", err))
}
}
runner.GoplsPath = goplsPath
runner.goplsPath = goplsPath
}
dir, err := ioutil.TempDir("", "gopls-regtest-")
if err != nil {
panic(fmt.Errorf("creating regtest temp directory: %v", err))
}
runner.TempDir = dir
runner.tempDir = dir
code := m.Run()
if err := runner.Close(); err != nil {

View File

@ -8,6 +8,7 @@ import (
"bytes"
"context"
"fmt"
"go/token"
"io"
"io/ioutil"
"net"
@ -29,24 +30,58 @@ import (
"golang.org/x/tools/internal/lsp/lsprpc"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/memoize"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/internal/xcontext"
)
// Mode is a bitmask that defines for which execution modes a test should run.
//
// Each mode controls several aspects of gopls' configuration:
// - Which server options to use for gopls sessions
// - Whether to use a shared cache
// - Whether to use a shared server
// - Whether to run the server in-process or in a separate process
//
// The behavior of each mode with respect to these aspects is summarized below.
// TODO(rfindley, cleanup): rather than using arbitrary names for these modes,
// we can compose them explicitly out of the features described here, allowing
// individual tests more freedom in constructing problematic execution modes.
// For example, a test could assert on a certain behavior when running with
// experimental options on a separate process. Moreover, we could unify 'Modes'
// with 'Options', and use RunMultiple rather than a hard-coded loop through
// modes.
//
// Mode | Options | Shared Cache? | Shared Server? | In-process?
// ---------------------------------------------------------------------------
// Default | Default | Y | N | Y
// Forwarded | Default | Y | Y | Y
// SeparateProcess | Default | Y | Y | N
// Experimental | Experimental | N | N | Y
type Mode int
const (
// Singleton mode uses a separate in-process gopls instance for each test,
// and communicates over pipes to mimic the gopls sidecar execution mode,
// which communicates over stdin/stderr.
Singleton Mode = 1 << iota
// Forwarded forwards connections to a shared in-process gopls instance.
// Default mode runs gopls with the default options, communicating over pipes
// to emulate the lsp sidecar execution mode, which communicates over
// stdin/stdout.
//
// It uses separate servers for each test, but a shared cache, to avoid
// duplicating work when processing GOROOT.
Default Mode = 1 << iota
// Forwarded uses the default options, but forwards connections to a shared
// in-process gopls server.
Forwarded
// SeparateProcess forwards connection to a shared separate gopls process.
// SeparateProcess uses the default options, but forwards connection to an
// external gopls daemon.
SeparateProcess
// Experimental enables all of the experimental configurations that are
// being developed.
// being developed, and runs gopls in sidecar mode.
//
// It uses a separate cache for each test, to exercise races that may only
// appear with cache misses.
Experimental
)
@ -55,14 +90,20 @@ const (
// remote), any tests that execute on the same Runner will share the same
// state.
type Runner struct {
DefaultModes Mode
Timeout time.Duration
GoplsPath string
PrintGoroutinesOnFailure bool
TempDir string
SkipCleanup bool
OptionsHook func(*source.Options)
// Configuration
DefaultModes Mode // modes to run for each test
Timeout time.Duration // per-test timeout, if set
PrintGoroutinesOnFailure bool // whether to dump goroutines on test failure
SkipCleanup bool // if set, don't delete test data directories when the test exits
OptionsHook func(*source.Options) // if set, use these options when creating gopls sessions
// Immutable state shared across test invocations
goplsPath string // path to the gopls executable (for SeparateProcess mode)
tempDir string // shared parent temp directory
fset *token.FileSet // shared FileSet
store *memoize.Store // shared store
// Lazily allocated resources
mu sync.Mutex
ts *servertest.TCPServer
socketDir string
@ -193,7 +234,7 @@ func InGOPATH() RunOption {
}
// DebugAddress configures a debug server bound to addr. This option is
// currently only supported when executing in Singleton mode. It is intended to
// currently only supported when executing in Default mode. It is intended to
// be used for long-running stress tests.
func DebugAddress(addr string) RunOption {
return optionSetter(func(opts *runConfig) {
@ -252,10 +293,10 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
mode Mode
getServer func(*testing.T, func(*source.Options)) jsonrpc2.StreamServer
}{
{"singleton", Singleton, singletonServer},
{"default", Default, r.defaultServer},
{"forwarded", Forwarded, r.forwardedServer},
{"separate_process", SeparateProcess, r.separateProcessServer},
{"experimental", Experimental, experimentalServer},
{"experimental", Experimental, r.experimentalServer},
}
for _, tc := range tests {
@ -267,10 +308,10 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
if config.modes&tc.mode == 0 {
continue
}
if config.debugAddr != "" && tc.mode != Singleton {
if config.debugAddr != "" && tc.mode != Default {
// Debugging is useful for running stress tests, but since the daemon has
// likely already been started, it would be too late to debug.
t.Fatalf("debugging regtest servers only works in Singleton mode, "+
t.Fatalf("debugging regtest servers only works in Default mode, "+
"got debug addr %q and mode %v", config.debugAddr, tc.mode)
}
@ -298,7 +339,7 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
di.MonitorMemory(ctx)
}
rootDir := filepath.Join(r.TempDir, filepath.FromSlash(t.Name()))
rootDir := filepath.Join(r.tempDir, filepath.FromSlash(t.Name()))
if err := os.MkdirAll(rootDir, 0755); err != nil {
t.Fatal(err)
}
@ -434,11 +475,13 @@ func (s *loggingFramer) printBuffers(testname string, w io.Writer) {
fmt.Fprintf(os.Stderr, "#### End Gopls Test Logs for %q\n", testname)
}
func singletonServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
return lsprpc.NewStreamServer(cache.New(optsHook), false)
// defaultServer handles the Default execution mode.
func (r *Runner) defaultServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
return lsprpc.NewStreamServer(cache.New(r.fset, r.store, optsHook), false)
}
func experimentalServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
// experimentalServer handles the Experimental execution mode.
func (r *Runner) experimentalServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
options := func(o *source.Options) {
optsHook(o)
o.EnableAllExperiments()
@ -446,28 +489,23 @@ func experimentalServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.S
// source.Options.EnableAllExperiments, but we want to test it.
o.ExperimentalWorkspaceModule = true
}
return lsprpc.NewStreamServer(cache.New(options), false)
return lsprpc.NewStreamServer(cache.New(nil, nil, options), false)
}
func (r *Runner) forwardedServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
ts := r.getTestServer(optsHook)
return newForwarder("tcp", ts.Addr)
}
// getTestServer gets the shared test server instance to connect to, or creates
// one if it doesn't exist.
func (r *Runner) getTestServer(optsHook func(*source.Options)) *servertest.TCPServer {
r.mu.Lock()
defer r.mu.Unlock()
// forwardedServer handles the Forwarded execution mode.
func (r *Runner) forwardedServer(_ *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
if r.ts == nil {
r.mu.Lock()
ctx := context.Background()
ctx = debug.WithInstance(ctx, "", "off")
ss := lsprpc.NewStreamServer(cache.New(optsHook), false)
ss := lsprpc.NewStreamServer(cache.New(nil, nil, optsHook), false)
r.ts = servertest.NewTCPServer(ctx, ss, nil)
r.mu.Unlock()
}
return r.ts
return newForwarder("tcp", r.ts.Addr)
}
// separateProcessServer handles the SeparateProcess execution mode.
func (r *Runner) separateProcessServer(t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
// TODO(rfindley): can we use the autostart behavior here, instead of
// pre-starting the remote?
@ -498,21 +536,22 @@ func (r *Runner) getRemoteSocket(t *testing.T) string {
return filepath.Join(r.socketDir, daemonFile)
}
if r.GoplsPath == "" {
if r.goplsPath == "" {
t.Fatal("cannot run tests with a separate process unless a path to a gopls binary is configured")
}
var err error
r.socketDir, err = ioutil.TempDir(r.TempDir, "gopls-regtest-socket")
r.socketDir, err = ioutil.TempDir(r.tempDir, "gopls-regtest-socket")
if err != nil {
t.Fatalf("creating tempdir: %v", err)
}
socket := filepath.Join(r.socketDir, daemonFile)
args := []string{"serve", "-listen", "unix;" + socket, "-listen.timeout", "10s"}
cmd := exec.Command(r.GoplsPath, args...)
cmd := exec.Command(r.goplsPath, args...)
cmd.Env = append(os.Environ(), runTestAsGoplsEnvvar+"=true")
var stderr bytes.Buffer
cmd.Stderr = &stderr
go func() {
// TODO(rfindley): this is racy; we're returning before we know that the command is running.
if err := cmd.Run(); err != nil {
panic(fmt.Sprintf("error running external gopls: %v\nstderr:\n%s", err, stderr.String()))
}
@ -537,7 +576,7 @@ func (r *Runner) Close() error {
}
}
if !r.SkipCleanup {
if err := os.RemoveAll(r.TempDir); err != nil {
if err := os.RemoveAll(r.tempDir); err != nil {
errmsgs = append(errmsgs, err.Error())
}
}

View File

@ -49,7 +49,7 @@ type runner struct {
func testSource(t *testing.T, datum *tests.Data) {
ctx := tests.Context(t)
cache := cache.New(nil)
cache := cache.New(nil, nil, nil)
session := cache.NewSession(ctx)
options := source.DefaultOptions().Clone()
tests.DefaultOptions(options)

View File

@ -236,12 +236,34 @@ func (p *Promise) wait(ctx context.Context) (interface{}, error) {
}
}
// An EvictionPolicy controls the eviction behavior of keys in a Store when
// they no longer have any references.
type EvictionPolicy int
const (
// ImmediatelyEvict evicts keys as soon as they no longer have references.
ImmediatelyEvict EvictionPolicy = iota
// NeverEvict does not evict keys.
NeverEvict
)
// A Store maps arbitrary keys to reference-counted promises.
//
// The zero value is a valid Store, though a store may also be created via
// NewStore if a custom EvictionPolicy is required.
type Store struct {
evictionPolicy EvictionPolicy
promisesMu sync.Mutex
promises map[interface{}]*Promise
}
// NewStore creates a new store with the given eviction policy.
func NewStore(policy EvictionPolicy) *Store {
return &Store{evictionPolicy: policy}
}
// Promise returns a reference-counted promise for the future result of
// calling the specified function.
//
@ -264,7 +286,9 @@ func (store *Store) Promise(key interface{}, function Function) (*Promise, func(
store.promisesMu.Unlock()
release := func() {
if atomic.AddInt32(&p.refcount, -1) == 0 {
// TODO(rfindley): this looks racy: it's possible that the refcount is
// incremented before we grab the lock.
if atomic.AddInt32(&p.refcount, -1) == 0 && store.evictionPolicy != NeverEvict {
store.promisesMu.Lock()
delete(store.promises, key)
store.promisesMu.Unlock()