From 6fb8e9cf78f462eb74c90f66eb9864eae01a5ad9 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Tue, 1 Aug 2023 16:06:19 +0100 Subject: [PATCH 1/8] compiler: fix compiledModule leak Fixes #1600. Signed-off-by: Nuno Cruces --- internal/engine/compiler/engine_cache.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/engine/compiler/engine_cache.go b/internal/engine/compiler/engine_cache.go index e6b3b0e914..db40243dd4 100644 --- a/internal/engine/compiler/engine_cache.go +++ b/internal/engine/compiler/engine_cache.go @@ -17,6 +17,11 @@ import ( func (e *engine) deleteCompiledModule(module *wasm.Module) { e.mux.Lock() defer e.mux.Unlock() + + cm := e.codes[module.ID] + for j := range cm.functions { + cm.functions[j].parent = nil + } delete(e.codes, module.ID) // Note: we do not call e.Cache.Delete, as the lifetime of From 41d51e973acc0023394f158e338427f25d6089ca Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Tue, 1 Aug 2023 14:13:06 -0700 Subject: [PATCH 2/8] break cyclic reference preventing finalizers from running Signed-off-by: Achille Roussel --- .../compiler/compiler_controlflow_test.go | 4 +- internal/engine/compiler/compiler_test.go | 6 +- internal/engine/compiler/engine.go | 37 +++++++----- internal/engine/compiler/engine_bench_test.go | 2 +- internal/engine/compiler/engine_cache.go | 13 +++-- internal/engine/compiler/engine_cache_test.go | 56 ++++++++++++------- internal/engine/compiler/engine_test.go | 14 +++-- internal/engine/compiler/impl_arm64_test.go | 2 +- 8 files changed, 81 insertions(+), 53 deletions(-) diff --git a/internal/engine/compiler/compiler_controlflow_test.go b/internal/engine/compiler/compiler_controlflow_test.go index 278cb68fed..132a25ecb4 100644 --- a/internal/engine/compiler/compiler_controlflow_test.go +++ b/internal/engine/compiler/compiler_controlflow_test.go @@ -822,7 +822,7 @@ func TestCompiler_callIndirect_largeTypeIndex(t *testing.T) { makeExecutable(code1.Bytes()) f := function{ - parent: &compiledFunction{parent: &compiledModule{executable: code1}}, + parent: &compiledFunction{parent: &compiledCode{executable: code1}}, codeInitialAddress: uintptr(unsafe.Pointer(&code1.Bytes()[0])), moduleInstance: env.moduleInstance, } @@ -896,7 +896,7 @@ func TestCompiler_compileCall(t *testing.T) { makeExecutable(code.Bytes()) me.functions = append(me.functions, function{ - parent: &compiledFunction{parent: &compiledModule{executable: code}}, + parent: &compiledFunction{parent: &compiledCode{executable: code}}, codeInitialAddress: uintptr(unsafe.Pointer(&code.Bytes()[0])), moduleInstance: env.moduleInstance, }) diff --git a/internal/engine/compiler/compiler_test.go b/internal/engine/compiler/compiler_test.go index 995b41cf3a..285300d7ec 100644 --- a/internal/engine/compiler/compiler_test.go +++ b/internal/engine/compiler/compiler_test.go @@ -202,7 +202,7 @@ func (j *compilerEnv) callEngine() *callEngine { } func (j *compilerEnv) exec(machineCode []byte) { - cm := new(compiledModule) + cm := &compiledModule{compiledCode: &compiledCode{}} if err := cm.executable.Map(len(machineCode)); err != nil { panic(err) } @@ -211,7 +211,7 @@ func (j *compilerEnv) exec(machineCode []byte) { makeExecutable(executable) f := &function{ - parent: &compiledFunction{parent: cm}, + parent: &compiledFunction{parent: cm.compiledCode}, codeInitialAddress: uintptr(unsafe.Pointer(&executable[0])), moduleInstance: j.moduleInstance, } @@ -268,7 +268,7 @@ func newCompilerEnvironment() *compilerEnv { Globals: []*wasm.GlobalInstance{}, Engine: me, }, - ce: me.newCallEngine(initialStackSize, &function{parent: &compiledFunction{parent: &compiledModule{}}}), + ce: me.newCallEngine(initialStackSize, &function{parent: &compiledFunction{parent: &compiledCode{}}}), } } diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index ca07a716d2..8c8c09e182 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -264,9 +264,23 @@ type ( } compiledModule struct { - executable asm.CodeSegment - functions []compiledFunction + // The data that need to be accessed by compiledFunction.parent are + // separated in an embedded field because we use finalizers to manage + // the lifecycle of compiledModule instances and having cyclic pointers + // prevents the Go runtime from calling them, which results in memory + // leaks since the memory mapped code segments cannot be released. + // + // The indirection guarantees that the finalizer set on compiledModule + // instances can run when all references are gone, and the Go GC can + // manage to reclaim the compiledCode when all compiledFunction objects + // referencing it have been freed. + *compiledCode + functions []compiledFunction + } + + compiledCode struct { source *wasm.Module + executable asm.CodeSegment ensureTermination bool } @@ -282,7 +296,7 @@ type ( index wasm.Index goFunc interface{} listener experimental.FunctionListener - parent *compiledModule + parent *compiledCode sourceOffsetMap sourceOffsetMap } @@ -496,13 +510,6 @@ func (e *engine) Close() (err error) { e.mux.Lock() defer e.mux.Unlock() // Releasing the references to compiled codes including the memory-mapped machine codes. - - for i := range e.codes { - for j := range e.codes[i].functions { - e.codes[i].functions[j].parent = nil - } - } - e.codes = nil return } @@ -523,9 +530,11 @@ func (e *engine) CompileModule(_ context.Context, module *wasm.Module, listeners var withGoFunc bool localFuncs, importedFuncs := len(module.FunctionSection), module.ImportFunctionCount cm := &compiledModule{ - functions: make([]compiledFunction, localFuncs), - ensureTermination: ensureTermination, - source: module, + compiledCode: &compiledCode{ + source: module, + ensureTermination: ensureTermination, + }, + functions: make([]compiledFunction, localFuncs), } if localFuncs == 0 { @@ -559,7 +568,7 @@ func (e *engine) CompileModule(_ context.Context, module *wasm.Module, listeners funcIndex := wasm.Index(i) compiledFn := &cm.functions[i] compiledFn.executableOffset = executable.Size() - compiledFn.parent = cm + compiledFn.parent = cm.compiledCode compiledFn.index = importedFuncs + funcIndex if i < ln { compiledFn.listener = listeners[i] diff --git a/internal/engine/compiler/engine_bench_test.go b/internal/engine/compiler/engine_bench_test.go index 991a98aca2..39bac99e69 100644 --- a/internal/engine/compiler/engine_bench_test.go +++ b/internal/engine/compiler/engine_bench_test.go @@ -20,7 +20,7 @@ func BenchmarkCallEngine_builtinFunctionFunctionListener(b *testing.B) { }, }, index: 0, - parent: &compiledModule{ + parent: &compiledCode{ source: &wasm.Module{ TypeSection: []wasm.FunctionType{{}}, FunctionSection: []wasm.Index{0}, diff --git a/internal/engine/compiler/engine_cache.go b/internal/engine/compiler/engine_cache.go index db40243dd4..d55a5844a4 100644 --- a/internal/engine/compiler/engine_cache.go +++ b/internal/engine/compiler/engine_cache.go @@ -18,10 +18,6 @@ func (e *engine) deleteCompiledModule(module *wasm.Module) { e.mux.Lock() defer e.mux.Unlock() - cm := e.codes[module.ID] - for j := range cm.functions { - cm.functions[j].parent = nil - } delete(e.codes, module.ID) // Note: we do not call e.Cache.Delete, as the lifetime of @@ -163,14 +159,19 @@ func deserializeCompiledModule(wazeroVersion string, reader io.ReadCloser, modul ensureTermination := header[cachedVersionEnd] != 0 functionsNum := binary.LittleEndian.Uint32(header[len(header)-4:]) - cm = &compiledModule{functions: make([]compiledFunction, functionsNum), ensureTermination: ensureTermination} + cm = &compiledModule{ + compiledCode: &compiledCode{ + ensureTermination: ensureTermination, + }, + functions: make([]compiledFunction, functionsNum), + } imported := module.ImportFunctionCount var eightBytes [8]byte for i := uint32(0); i < functionsNum; i++ { f := &cm.functions[i] - f.parent = cm + f.parent = cm.compiledCode // Read the stack pointer ceil. if f.stackPointerCeil, err = readUint64(reader, &eightBytes); err != nil { diff --git a/internal/engine/compiler/engine_cache_test.go b/internal/engine/compiler/engine_cache_test.go index a2ad605657..955693ee00 100644 --- a/internal/engine/compiler/engine_cache_test.go +++ b/internal/engine/compiler/engine_cache_test.go @@ -38,7 +38,9 @@ func TestSerializeCompiledModule(t *testing.T) { }{ { in: &compiledModule{ - executable: makeCodeSegment(1, 2, 3, 4, 5), + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3, 4, 5), + }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345}, }, @@ -57,8 +59,10 @@ func TestSerializeCompiledModule(t *testing.T) { }, { in: &compiledModule{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5), + compiledCode: &compiledCode{ + ensureTermination: true, + executable: makeCodeSegment(1, 2, 3, 4, 5), + }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345}, }, @@ -77,8 +81,10 @@ func TestSerializeCompiledModule(t *testing.T) { }, { in: &compiledModule{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5, 1, 2, 3), + compiledCode: &compiledCode{ + ensureTermination: true, + executable: makeCodeSegment(1, 2, 3, 4, 5, 1, 2, 3), + }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345}, {executableOffset: 5, stackPointerCeil: 0xffffffff}, @@ -159,7 +165,9 @@ func TestDeserializeCompiledModule(t *testing.T) { []byte{1, 2, 3, 4, 5}, // machine code. ), expCompiledModule: &compiledModule{ - executable: makeCodeSegment(1, 2, 3, 4, 5), + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3, 4, 5), + }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345, index: 0}, }, @@ -181,9 +189,11 @@ func TestDeserializeCompiledModule(t *testing.T) { []byte{1, 2, 3, 4, 5}, // code. ), expCompiledModule: &compiledModule{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5), - functions: []compiledFunction{{executableOffset: 0, stackPointerCeil: 12345, index: 0}}, + compiledCode: &compiledCode{ + ensureTermination: true, + executable: makeCodeSegment(1, 2, 3, 4, 5), + }, + functions: []compiledFunction{{executableOffset: 0, stackPointerCeil: 12345, index: 0}}, }, expStaleCache: false, expErr: "", @@ -208,7 +218,9 @@ func TestDeserializeCompiledModule(t *testing.T) { ), importedFunctionCount: 1, expCompiledModule: &compiledModule{ - executable: makeCodeSegment(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), + }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345, index: 1}, {executableOffset: 7, stackPointerCeil: 0xffffffff, index: 2}, @@ -279,8 +291,8 @@ func TestDeserializeCompiledModule(t *testing.T) { if tc.expCompiledModule != nil { require.Equal(t, len(tc.expCompiledModule.functions), len(cm.functions)) for i := 0; i < len(cm.functions); i++ { - require.Equal(t, cm, cm.functions[i].parent) - tc.expCompiledModule.functions[i].parent = cm + require.Equal(t, cm.compiledCode, cm.functions[i].parent) + tc.expCompiledModule.functions[i].parent = cm.compiledCode } } @@ -361,13 +373,13 @@ func TestEngine_getCompiledModuleFromCache(t *testing.T) { }, expHit: true, expCompiledModule: &compiledModule{ - executable: makeCodeSegment(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), + }, functions: []compiledFunction{ {stackPointerCeil: 12345, executableOffset: 0, index: 0}, {stackPointerCeil: 0xffffffff, executableOffset: 5, index: 1}, }, - source: nil, - ensureTermination: false, }, }, } @@ -379,7 +391,7 @@ func TestEngine_getCompiledModuleFromCache(t *testing.T) { if exp := tc.expCompiledModule; exp != nil { exp.source = m for i := range tc.expCompiledModule.functions { - tc.expCompiledModule.functions[i].parent = exp + tc.expCompiledModule.functions[i].parent = exp.compiledCode } } @@ -422,8 +434,10 @@ func TestEngine_addCompiledModuleToCache(t *testing.T) { tc := filecache.New(t.TempDir()) e := engine{fileCache: tc} cm := &compiledModule{ - executable: makeCodeSegment(1, 2, 3), - functions: []compiledFunction{{stackPointerCeil: 123}}, + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3), + }, + functions: []compiledFunction{{stackPointerCeil: 123}}, } m := &wasm.Module{ID: sha256.Sum256(nil), IsHostModule: true} // Host module! err := e.addCompiledModuleToCache(m, cm) @@ -438,8 +452,10 @@ func TestEngine_addCompiledModuleToCache(t *testing.T) { e := engine{fileCache: tc} m := &wasm.Module{} cm := &compiledModule{ - executable: makeCodeSegment(1, 2, 3), - functions: []compiledFunction{{stackPointerCeil: 123}}, + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2, 3), + }, + functions: []compiledFunction{{stackPointerCeil: 123}}, } err := e.addCompiledModuleToCache(m, cm) require.NoError(t, err) diff --git a/internal/engine/compiler/engine_test.go b/internal/engine/compiler/engine_test.go index 520b35f93e..e18ace6239 100644 --- a/internal/engine/compiler/engine_test.go +++ b/internal/engine/compiler/engine_test.go @@ -234,7 +234,9 @@ func TestCompiler_CompileModule(t *testing.T) { func TestCompiler_Releasecode_Panic(t *testing.T) { captured := require.CapturePanic(func() { releaseCompiledModule(&compiledModule{ - executable: makeCodeSegment(1, 2), + compiledCode: &compiledCode{ + executable: makeCodeSegment(1, 2), + }, }) }) require.Contains(t, captured.Error(), "compiler: failed to munmap code segment") @@ -392,15 +394,15 @@ func TestCallEngine_deferredOnCall(t *testing.T) { } f1 := &function{ funcType: &wasm.FunctionType{ParamNumInUint64: 2}, - parent: &compiledFunction{parent: &compiledModule{source: s}, index: 0}, + parent: &compiledFunction{parent: &compiledCode{source: s}, index: 0}, } f2 := &function{ funcType: &wasm.FunctionType{ParamNumInUint64: 2, ResultNumInUint64: 3}, - parent: &compiledFunction{parent: &compiledModule{source: s}, index: 1}, + parent: &compiledFunction{parent: &compiledCode{source: s}, index: 1}, } f3 := &function{ funcType: &wasm.FunctionType{ResultNumInUint64: 1}, - parent: &compiledFunction{parent: &compiledModule{source: s}, index: 2}, + parent: &compiledFunction{parent: &compiledCode{source: s}, index: 2}, } ce := &callEngine{ @@ -598,7 +600,7 @@ func TestCallEngine_builtinFunctionFunctionListenerBefore(t *testing.T) { }, }, index: 0, - parent: &compiledModule{source: &wasm.Module{ + parent: &compiledCode{source: &wasm.Module{ FunctionSection: []wasm.Index{0}, CodeSection: []wasm.Code{{}}, TypeSection: []wasm.FunctionType{{}}, @@ -624,7 +626,7 @@ func TestCallEngine_builtinFunctionFunctionListenerAfter(t *testing.T) { }, }, index: 0, - parent: &compiledModule{source: &wasm.Module{ + parent: &compiledCode{source: &wasm.Module{ FunctionSection: []wasm.Index{0}, CodeSection: []wasm.Code{{}}, TypeSection: []wasm.FunctionType{{}}, diff --git a/internal/engine/compiler/impl_arm64_test.go b/internal/engine/compiler/impl_arm64_test.go index 315f0c65e2..da59b58f59 100644 --- a/internal/engine/compiler/impl_arm64_test.go +++ b/internal/engine/compiler/impl_arm64_test.go @@ -42,7 +42,7 @@ func TestArm64Compiler_indirectCallWithTargetOnCallingConvReg(t *testing.T) { makeExecutable(executable) f := function{ - parent: &compiledFunction{parent: &compiledModule{executable: code}}, + parent: &compiledFunction{parent: &compiledCode{executable: code}}, codeInitialAddress: code.Addr(), moduleInstance: env.moduleInstance, } From a1fc1dbcc6c45fd14a016c07688d4311026793c4 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Wed, 2 Aug 2023 00:05:17 +0100 Subject: [PATCH 3/8] fix amd64 compile issue Signed-off-by: Nuno Cruces --- internal/engine/compiler/impl_amd64_test.go | 2 +- internal/gojs/compiler_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/engine/compiler/impl_amd64_test.go b/internal/engine/compiler/impl_amd64_test.go index ae7d08fd9c..316bfceb77 100644 --- a/internal/engine/compiler/impl_amd64_test.go +++ b/internal/engine/compiler/impl_amd64_test.go @@ -44,7 +44,7 @@ func TestAmd64Compiler_indirectCallWithTargetOnCallingConvReg(t *testing.T) { makeExecutable(executable) f := function{ - parent: &compiledFunction{parent: &compiledModule{executable: code}}, + parent: &compiledFunction{parent: &compiledCode{executable: code}}, codeInitialAddress: code.Addr(), moduleInstance: env.moduleInstance, typeID: 0, diff --git a/internal/gojs/compiler_test.go b/internal/gojs/compiler_test.go index ff83a9d363..50ab9009d6 100644 --- a/internal/gojs/compiler_test.go +++ b/internal/gojs/compiler_test.go @@ -83,7 +83,7 @@ var ( func TestMain(m *testing.M) { // For some reason, windows and freebsd fail to compile with exit status 1. - if o := runtime.GOOS; o != "darwin" && o != "linux" { + if o := runtime.GOOS; o != "linux" { log.Println("gojs: skipping due to not yet supported OS:", o) os.Exit(0) } From 257a75b271e78c31ffdbe1dd742c3e454b5ebdc2 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Wed, 2 Aug 2023 00:19:11 +0100 Subject: [PATCH 4/8] revert skipping failing test Signed-off-by: Nuno Cruces --- internal/gojs/compiler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gojs/compiler_test.go b/internal/gojs/compiler_test.go index 50ab9009d6..ff83a9d363 100644 --- a/internal/gojs/compiler_test.go +++ b/internal/gojs/compiler_test.go @@ -83,7 +83,7 @@ var ( func TestMain(m *testing.M) { // For some reason, windows and freebsd fail to compile with exit status 1. - if o := runtime.GOOS; o != "linux" { + if o := runtime.GOOS; o != "darwin" && o != "linux" { log.Println("gojs: skipping due to not yet supported OS:", o) os.Exit(0) } From 3ce657b532d171031eaa9b00b350530b2b5056b8 Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Tue, 1 Aug 2023 17:23:49 -0700 Subject: [PATCH 5/8] keep references to compiledModule in moduleEngine and callEngine Signed-off-by: Achille Roussel --- internal/engine/compiler/engine.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index 8c8c09e182..a0385910a7 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -48,6 +48,10 @@ type ( // as the underlying memory region is accessed by assembly directly by using // codesElement0Address. functions []function + + // Keep a reference to the compiled module to prevent the GC from reclaiming + // it while the code may still be needed. + module *compiledModule } // callEngine holds context per moduleEngine.Call, and shared across all the @@ -130,6 +134,10 @@ type ( // initialFn is the initial function for this call engine. initialFn *function + // Keep a reference to the compiled module to prevent the GC from reclaiming + // it while the code may still be needed. + module *compiledModule + // stackIterator provides a way to iterate over the stack for Listeners. // It is setup and valid only during a call to a Listener hook. stackIterator stackIterator @@ -637,6 +645,8 @@ func (e *engine) NewModuleEngine(module *wasm.Module, instance *wasm.ModuleInsta parent: c, } } + + me.module = cm return me, nil } @@ -769,6 +779,9 @@ func (ce *callEngine) call(ctx context.Context, params, results []uint64) (_ []u results = make([]uint64, ft.ResultNumInUint64) } copy(results, ce.stack) + + // Ensure that the compiled module will never be GC'd before this method returns. + runtime.KeepAlive(ce.module) return results, nil } @@ -973,6 +986,7 @@ func (e *moduleEngine) newCallEngine(stackSize uint64, fn *function) *callEngine initialFn: fn, moduleContext: moduleContext{fn: fn}, ensureTermination: fn.parent.parent.ensureTermination, + module: e.module, } stackHeader := (*reflect.SliceHeader)(unsafe.Pointer(&ce.stack)) From 15867bff5ba2677fa847c3d49c6a9e87e56d5a9b Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Tue, 1 Aug 2023 17:31:43 -0700 Subject: [PATCH 6/8] remove callEngine.ensureTermination since we can now access the field from compiledModule Signed-off-by: Achille Roussel --- internal/engine/compiler/engine.go | 30 +++++++++---------- internal/engine/compiler/engine_cache.go | 7 ++--- internal/engine/compiler/engine_cache_test.go | 14 ++++----- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index a0385910a7..a5ff0ee883 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -141,8 +141,6 @@ type ( // stackIterator provides a way to iterate over the stack for Listeners. // It is setup and valid only during a call to a Listener hook. stackIterator stackIterator - - ensureTermination bool } // moduleContext holds the per-function call specific module information. @@ -284,12 +282,13 @@ type ( // referencing it have been freed. *compiledCode functions []compiledFunction + + ensureTermination bool } compiledCode struct { - source *wasm.Module - executable asm.CodeSegment - ensureTermination bool + source *wasm.Module + executable asm.CodeSegment } // compiledFunction corresponds to a function in a module (not instantiated one). This holds the machine code @@ -539,10 +538,10 @@ func (e *engine) CompileModule(_ context.Context, module *wasm.Module, listeners localFuncs, importedFuncs := len(module.FunctionSection), module.ImportFunctionCount cm := &compiledModule{ compiledCode: &compiledCode{ - source: module, - ensureTermination: ensureTermination, + source: module, }, - functions: make([]compiledFunction, localFuncs), + functions: make([]compiledFunction, localFuncs), + ensureTermination: ensureTermination, } if localFuncs == 0 { @@ -739,7 +738,7 @@ func (ce *callEngine) CallWithStack(ctx context.Context, stack []uint64) error { func (ce *callEngine) call(ctx context.Context, params, results []uint64) (_ []uint64, err error) { m := ce.initialFn.moduleInstance - if ce.ensureTermination { + if ce.module.ensureTermination { select { case <-ctx.Done(): // If the provided context is already done, close the call context @@ -765,7 +764,7 @@ func (ce *callEngine) call(ctx context.Context, params, results []uint64) (_ []u ft := ce.initialFn.funcType ce.initializeStack(ft, params) - if ce.ensureTermination { + if ce.module.ensureTermination { done := m.CloseModuleOnCanceledOrTimeout(ctx) defer done() } @@ -981,12 +980,11 @@ var initialStackSize uint64 = 512 func (e *moduleEngine) newCallEngine(stackSize uint64, fn *function) *callEngine { ce := &callEngine{ - stack: make([]uint64, stackSize), - archContext: newArchContext(), - initialFn: fn, - moduleContext: moduleContext{fn: fn}, - ensureTermination: fn.parent.parent.ensureTermination, - module: e.module, + stack: make([]uint64, stackSize), + archContext: newArchContext(), + initialFn: fn, + moduleContext: moduleContext{fn: fn}, + module: e.module, } stackHeader := (*reflect.SliceHeader)(unsafe.Pointer(&ce.stack)) diff --git a/internal/engine/compiler/engine_cache.go b/internal/engine/compiler/engine_cache.go index d55a5844a4..37e481bdb6 100644 --- a/internal/engine/compiler/engine_cache.go +++ b/internal/engine/compiler/engine_cache.go @@ -160,10 +160,9 @@ func deserializeCompiledModule(wazeroVersion string, reader io.ReadCloser, modul ensureTermination := header[cachedVersionEnd] != 0 functionsNum := binary.LittleEndian.Uint32(header[len(header)-4:]) cm = &compiledModule{ - compiledCode: &compiledCode{ - ensureTermination: ensureTermination, - }, - functions: make([]compiledFunction, functionsNum), + compiledCode: new(compiledCode), + functions: make([]compiledFunction, functionsNum), + ensureTermination: ensureTermination, } imported := module.ImportFunctionCount diff --git a/internal/engine/compiler/engine_cache_test.go b/internal/engine/compiler/engine_cache_test.go index 955693ee00..c26433a316 100644 --- a/internal/engine/compiler/engine_cache_test.go +++ b/internal/engine/compiler/engine_cache_test.go @@ -60,12 +60,12 @@ func TestSerializeCompiledModule(t *testing.T) { { in: &compiledModule{ compiledCode: &compiledCode{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5), + executable: makeCodeSegment(1, 2, 3, 4, 5), }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345}, }, + ensureTermination: true, }, exp: concat( []byte(wazeroMagic), @@ -82,13 +82,13 @@ func TestSerializeCompiledModule(t *testing.T) { { in: &compiledModule{ compiledCode: &compiledCode{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5, 1, 2, 3), + executable: makeCodeSegment(1, 2, 3, 4, 5, 1, 2, 3), }, functions: []compiledFunction{ {executableOffset: 0, stackPointerCeil: 12345}, {executableOffset: 5, stackPointerCeil: 0xffffffff}, }, + ensureTermination: true, }, exp: concat( []byte(wazeroMagic), @@ -190,10 +190,10 @@ func TestDeserializeCompiledModule(t *testing.T) { ), expCompiledModule: &compiledModule{ compiledCode: &compiledCode{ - ensureTermination: true, - executable: makeCodeSegment(1, 2, 3, 4, 5), + executable: makeCodeSegment(1, 2, 3, 4, 5), }, - functions: []compiledFunction{{executableOffset: 0, stackPointerCeil: 12345, index: 0}}, + functions: []compiledFunction{{executableOffset: 0, stackPointerCeil: 12345, index: 0}}, + ensureTermination: true, }, expStaleCache: false, expErr: "", From 3946e94b7fad146152f14abfbdeae6f13793e7ae Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Tue, 1 Aug 2023 17:37:03 -0700 Subject: [PATCH 7/8] fix position of runtime.KeepAlive in callEngine.call Signed-off-by: Achille Roussel --- internal/engine/compiler/engine.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go index a5ff0ee883..86abce4a51 100644 --- a/internal/engine/compiler/engine.go +++ b/internal/engine/compiler/engine.go @@ -759,6 +759,8 @@ func (ce *callEngine) call(ctx context.Context, params, results []uint64) (_ []u // If the module closed during the call, and the call didn't err for another reason, set an ExitError. err = m.FailIfClosed() } + // Ensure that the compiled module will never be GC'd before this method returns. + runtime.KeepAlive(ce.module) }() ft := ce.initialFn.funcType @@ -778,9 +780,6 @@ func (ce *callEngine) call(ctx context.Context, params, results []uint64) (_ []u results = make([]uint64, ft.ResultNumInUint64) } copy(results, ce.stack) - - // Ensure that the compiled module will never be GC'd before this method returns. - runtime.KeepAlive(ce.module) return results, nil } From 129e49ae84a1b635d767ff48246aefdb3015da19 Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Tue, 1 Aug 2023 18:00:51 -0700 Subject: [PATCH 8/8] test memory usage of the wazero engine Signed-off-by: Achille Roussel --- .../integration_test/engine/memleak_test.go | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 internal/integration_test/engine/memleak_test.go diff --git a/internal/integration_test/engine/memleak_test.go b/internal/integration_test/engine/memleak_test.go new file mode 100644 index 0000000000..4cb0179a3e --- /dev/null +++ b/internal/integration_test/engine/memleak_test.go @@ -0,0 +1,51 @@ +package adhoc + +import ( + "context" + "log" + "runtime" + "testing" + "time" + + "github.com/tetratelabs/wazero" +) + +func TestMemoryLeak(t *testing.T) { + if testing.Short() { + t.Skip("skipping memory leak test in short mode.") + } + + duration := 5 * time.Second + t.Logf("running memory leak test for %s", duration) + + ctx, cancel := context.WithTimeout(context.Background(), duration) + defer cancel() + + for ctx.Err() == nil { + if err := testMemoryLeakInstantiateRuntimeAndModule(); err != nil { + log.Panicln(err) + } + } + + var stats runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&stats) + + if stats.Alloc > (100 * 1024 * 1024) { + t.Errorf("wazero used more than 100 MiB after running the test for %s (alloc=%d)", duration, stats.Alloc) + } +} + +func testMemoryLeakInstantiateRuntimeAndModule() error { + ctx := context.Background() + + runtime := wazero.NewRuntime(ctx) + defer runtime.Close(ctx) + + mod, err := runtime.InstantiateWithConfig(ctx, memoryWasm, + wazero.NewModuleConfig().WithStartFunctions()) + if err != nil { + return err + } + return mod.Close(ctx) +}