Skip to content

Commit

Permalink
fix: Reset the module global cache on every new interpretation to avo…
Browse files Browse the repository at this point in the history
…id using outdated modules (#1291)

## Description:
Using a single module cache lead to issues like [this
one](#1226) b/c the
interpreter was pulling the outdated cache.

Fixes #1226 

## Is this change user facing?
NO
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
  • Loading branch information
Guillaume Bouvignies committed Sep 12, 2023
1 parent 33d867e commit 81c5462
Showing 1 changed file with 14 additions and 13 deletions.
Expand Up @@ -51,14 +51,13 @@ var (

type StartosisInterpreter struct {
// This is mutex protected as interpreting two different scripts in parallel could potentially cause
// problems with the moduleGlobalsCache & moduleContentProvider. Fixing this is quite complicated, which we decided not to do.
mutex *sync.Mutex
serviceNetwork service_network.ServiceNetwork
recipeExecutor *runtime_value_store.RuntimeValueStore
moduleGlobalsCache map[string]*startosis_packages.ModuleCacheEntry
starlarkValueSerde *kurtosis_types.StarlarkValueSerde
// problems with moduleContentProvider. Fixing this is quite complicated, which we decided not to do.
mutex *sync.Mutex
serviceNetwork service_network.ServiceNetwork
recipeExecutor *runtime_value_store.RuntimeValueStore
// TODO AUTH there will be a leak here in case people with different repo visibility access a module
moduleContentProvider startosis_packages.PackageContentProvider
starlarkValueSerde *kurtosis_types.StarlarkValueSerde
enclaveEnvVars string
}

Expand All @@ -69,7 +68,6 @@ func NewStartosisInterpreter(serviceNetwork service_network.ServiceNetwork, modu
mutex: &sync.Mutex{},
serviceNetwork: serviceNetwork,
recipeExecutor: runtimeValueStore,
moduleGlobalsCache: make(map[string]*startosis_packages.ModuleCacheEntry),
moduleContentProvider: moduleContentProvider,
enclaveEnvVars: enclaveVarEnvs,
starlarkValueSerde: starlarkValueSerde,
Expand Down Expand Up @@ -225,7 +223,10 @@ func (interpreter *StartosisInterpreter) Interpret(
if packageId != startosis_constants.PackageIdPlaceholderForStandaloneScript {
moduleLocator = path.Join(moduleLocator, relativePathtoMainFile)
}
globalVariables, interpretationErr := interpreter.interpretInternal(moduleLocator, serializedStarlark, newInstructionsPlan)

// we use a new cache for every interpretation b/c the content of the module might have changed
moduleGlobalCache := map[string]*startosis_packages.ModuleCacheEntry{}
globalVariables, interpretationErr := interpreter.interpretInternal(moduleLocator, serializedStarlark, newInstructionsPlan, moduleGlobalCache)
if interpretationErr != nil {
return startosis_constants.NoOutputObject, nil, interpretationErr.ToAPIType()
}
Expand Down Expand Up @@ -311,13 +312,13 @@ func (interpreter *StartosisInterpreter) Interpret(
return startosis_constants.NoOutputObject, newInstructionsPlan, nil
}

func (interpreter *StartosisInterpreter) interpretInternal(moduleLocator string, serializedStarlark string, instructionPlan *instructions_plan.InstructionsPlan) (starlark.StringDict, *startosis_errors.InterpretationError) {
func (interpreter *StartosisInterpreter) interpretInternal(moduleLocator string, serializedStarlark string, instructionPlan *instructions_plan.InstructionsPlan, moduleGlobalCache map[string]*startosis_packages.ModuleCacheEntry) (starlark.StringDict, *startosis_errors.InterpretationError) {
// We spin up a new thread for every call to interpreterInternal such that the stacktrace provided by the Starlark
// Go interpreter is relative to each individual thread, and we don't keep accumulating stacktrace entries from the
// previous calls inside the same thread
// The thread name is set to the locator of the module so that we can use it to resolve relative paths
thread := newStarlarkThread(moduleLocator)
predeclared, interpretationErr := interpreter.buildBindings(thread, instructionPlan)
predeclared, interpretationErr := interpreter.buildBindings(thread, instructionPlan, moduleGlobalCache)
if interpretationErr != nil {
return nil, interpretationErr
}
Expand All @@ -330,9 +331,9 @@ func (interpreter *StartosisInterpreter) interpretInternal(moduleLocator string,
return globalVariables, nil
}

func (interpreter *StartosisInterpreter) buildBindings(thread *starlark.Thread, instructionPlan *instructions_plan.InstructionsPlan) (*starlark.StringDict, *startosis_errors.InterpretationError) {
func (interpreter *StartosisInterpreter) buildBindings(thread *starlark.Thread, instructionPlan *instructions_plan.InstructionsPlan, moduleGlobalCache map[string]*startosis_packages.ModuleCacheEntry) (*starlark.StringDict, *startosis_errors.InterpretationError) {
recursiveInterpretForModuleLoading := func(moduleId string, serializedStartosis string) (starlark.StringDict, *startosis_errors.InterpretationError) {
result, err := interpreter.interpretInternal(moduleId, serializedStartosis, instructionPlan)
result, err := interpreter.interpretInternal(moduleId, serializedStartosis, instructionPlan, moduleGlobalCache)
if err != nil {
return nil, err
}
Expand All @@ -349,7 +350,7 @@ func (interpreter *StartosisInterpreter) buildBindings(thread *starlark.Thread,
predeclared[builtins.KurtosisModuleName] = kurtosisModule

// Add all Kurtosis helpers
for _, kurtosisHelper := range KurtosisHelpers(recursiveInterpretForModuleLoading, interpreter.moduleContentProvider, interpreter.moduleGlobalsCache) {
for _, kurtosisHelper := range KurtosisHelpers(recursiveInterpretForModuleLoading, interpreter.moduleContentProvider, moduleGlobalCache) {
predeclared[kurtosisHelper.Name()] = kurtosisHelper
}

Expand Down

0 comments on commit 81c5462

Please sign in to comment.