Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binary.decodeCode cause Memory leak ? #1600

Closed
panchen66 opened this issue Jul 29, 2023 · 13 comments · Fixed by #1608
Closed

binary.decodeCode cause Memory leak ? #1600

panchen66 opened this issue Jul 29, 2023 · 13 comments · Fixed by #1608
Labels
bug Something isn't working

Comments

@panchen66
Copy link

panchen66 commented Jul 29, 2023

Describe the bug
A clear and concise description of what the bug is.
I suspect make binary.decodeCode cause Memory leak

To Reproduce
I will reinitialize the instance when I catch wasm execution exceptions, but after continuous initialization, I find that memory continues to grow

Showing nodes accounting for 2560.42MB, 92.16% of 2778.31MB total
Dropped 223 nodes (cum <= 13.89MB)
Showing top 10 nodes out of 68
      flat  flat%   sum%        cum   cum%
 1231.40MB 44.32% 44.32%  1231.40MB 44.32%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeCode
  323.56MB 11.65% 55.97%   411.41MB 14.81%  github.com/tetratelabs/wazero/internal/engine/compiler.(*engine).CompileModule
  283.46MB 10.20% 66.17%   349.47MB 12.58%  github.com/tetratelabs/wazero/internal/wasm.(*Module).buildFunctionDefinitionsOnce
  194.70MB  7.01% 73.18%   195.20MB  7.03%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeDataSegment
  187.75MB  6.76% 79.94%  1419.15MB 51.08%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeCodeSection
   88.01MB  3.17% 83.10%    88.01MB  3.17%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeUTF8
   82.12MB  2.96% 86.06%    96.15MB  3.46%  github.com/tetratelabs/wazero/internal/wasm.addFuncs
   71.23MB  2.56% 88.62%   156.24MB  5.62%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeFunctionNames
   65.50MB  2.36% 90.98%    65.50MB  2.36%  strings.(*Builder).WriteString
   32.69MB  1.18% 92.16%   227.89MB  8.20%  github.com/tetratelabs/wazero/internal/wasm/binary.decodeDataSection
(pprof) list binary.decodeCode
Total: 2.71GB
ROUTINE ======================== github.com/tetratelabs/wazero/internal/wasm/binary.decodeCode in /Users/pc/go/pkg/mod/github.com/tetratelabs/[email protected]/internal/wasm/binary/code.go
    1.20GB     1.20GB (flat, cum) 44.32% of Total
         .          .     13:func decodeCode(r *bytes.Reader, codeSectionStart uint64, ret *wasm.Code) (err error) {
         .          .     14:   ss, _, err := leb128.DecodeUint32(r)
         .          .     15:   if err != nil {
         .          .     16:           return fmt.Errorf("get the size of code: %w", err)
         .          .     17:   }
         .          .     18:   remaining := int64(ss)
         .          .     19:
         .          .     20:   // Parse #locals.
         .          .     21:   ls, bytesRead, err := leb128.DecodeUint32(r)
         .          .     22:   remaining -= int64(bytesRead)
         .          .     23:   if err != nil {
         .          .     24:           return fmt.Errorf("get the size locals: %v", err)
         .          .     25:   } else if remaining < 0 {
         .          .     26:           return io.EOF
         .          .     27:   }
         .          .     28:
         .          .     29:   // Validate the locals.
         .          .     30:   bytesRead = 0
         .          .     31:   var sum uint64
         .          .     32:   for i := uint32(0); i < ls; i++ {
         .          .     33:           num, n, err := leb128.DecodeUint32(r)
         .          .     34:           if err != nil {
         .          .     35:                   return fmt.Errorf("read n of locals: %v", err)
         .          .     36:           } else if remaining < 0 {
         .          .     37:                   return io.EOF
         .          .     38:           }
         .          .     39:
         .          .     40:           sum += uint64(num)
         .          .     41:
         .          .     42:           b, err := r.ReadByte()
         .          .     43:           if err != nil {
         .          .     44:                   return fmt.Errorf("read type of local: %v", err)
         .          .     45:           }
         .          .     46:
         .          .     47:           bytesRead += n + 1
         .          .     48:           switch vt := b; vt {
         .          .     49:           case wasm.ValueTypeI32, wasm.ValueTypeF32, wasm.ValueTypeI64, wasm.ValueTypeF64,
         .          .     50:                   wasm.ValueTypeFuncref, wasm.ValueTypeExternref, wasm.ValueTypeV128:
         .          .     51:           default:
         .          .     52:                   return fmt.Errorf("invalid local type: 0x%x", vt)
         .          .     53:           }
         .          .     54:   }
         .          .     55:
         .          .     56:   if sum > math.MaxUint32 {
         .          .     57:           return fmt.Errorf("too many locals: %d", sum)
         .          .     58:   }
         .          .     59:
         .          .     60:   // Rewind the buffer.
         .          .     61:   _, err = r.Seek(-int64(bytesRead), io.SeekCurrent)
         .          .     62:   if err != nil {
         .          .     63:           return err
         .          .     64:   }
         .          .     65:
   18.51MB    18.51MB     66:   localTypes := make([]wasm.ValueType, 0, sum)
         .          .     67:   for i := uint32(0); i < ls; i++ {
         .          .     68:           num, bytesRead, err := leb128.DecodeUint32(r)
         .          .     69:           remaining -= int64(bytesRead) + 1 // +1 for the subsequent ReadByte
         .          .     70:           if err != nil {
         .          .     71:                   return fmt.Errorf("read n of locals: %v", err)
         .          .     72:           } else if remaining < 0 {
         .          .     73:                   return io.EOF
         .          .     74:           }
         .          .     75:
         .          .     76:           b, err := r.ReadByte()
         .          .     77:           if err != nil {
         .          .     78:                   return fmt.Errorf("read type of local: %v", err)
         .          .     79:           }
         .          .     80:
         .          .     81:           for j := uint32(0); j < num; j++ {
         .          .     82:                   localTypes = append(localTypes, b)
         .          .     83:           }
         .          .     84:   }
         .          .     85:
         .          .     86:   bodyOffsetInCodeSection := codeSectionStart - uint64(r.Len())
    1.18GB     1.18GB     87:   body := make([]byte, remaining)
         .          .     88:   if _, err = io.ReadFull(r, body); err != nil {
         .          .     89:           return fmt.Errorf("read body: %w", err)
         .          .     90:   }
         .          .     91:
         .          .     92:   if endIndex := len(body) - 1; endIndex < 0 || body[endIndex] != wasm.OpcodeEnd {
ROUTINE ======================== github.com/tetratelabs/wazero/internal/wasm/binary.decodeCodeSection in /Users/pc/go/pkg/mod/github.com/tetratelabs/[email protected]/internal/wasm/binary/section.go
  187.75MB     1.39GB (flat, cum) 51.08% of Total
         .          .    186:func decodeCodeSection(r *bytes.Reader) ([]wasm.Code, error) {
         .          .    187:   codeSectionStart := uint64(r.Len())
         .          .    188:   vs, _, err := leb128.DecodeUint32(r)
         .          .    189:   if err != nil {
         .          .    190:           return nil, fmt.Errorf("get size of vector: %w", err)
         .          .    191:   }
         .          .    192:
  187.75MB   187.75MB    193:   result := make([]wasm.Code, vs)
         .          .    194:   for i := uint32(0); i < vs; i++ {
         .     1.20GB    195:           err = decodeCode(r, codeSectionStart, &result[i])
         .          .    196:           if err != nil {
         .          .    197:                   return nil, fmt.Errorf("read %d-th code segment: %v", i, err)
         .          .    198:           }
         .          .    199:   }
         .          .    200:   return result, nil
(pprof) 

I debugged the shutdown code and it seems that there was no recycling of the source wasm object. How should I recycle this object

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please complete the relevant information):

  • Go version: [e.g. 1.18.1]
  • wazero Version: [e.g. c815060]
  • Host architecture: [e.g. amd64]
  • Runtime mode: [e.g. interpreter or compiler]

Additional context
Add any other context about the problem here.

@panchen66 panchen66 added the bug Something isn't working label Jul 29, 2023
@mathetake mathetake added question Further information is requested and removed bug Something isn't working labels Jul 29, 2023
@mathetake
Copy link
Member

What do you mean by “continuous initialization”? Do you want to be more specific when raising an issue like with a reproducible code snipe?

@mathetake
Copy link
Member

If you are not closing the compiled module, sure it will increase the memory usage and that’s totally expected

@panchen66
Copy link
Author

“continuous initialization” , I will call runtime.Instantiate when the function loops dead. and i call api.Module.Close to closing the compiled module, but binary.decodeCode continuously growing.
Currently, I have two questions: 1. How to handle dead loop functions 2. How to close instances, including binary. decodeCode

Looking forward to your reply very much

@panchen66
Copy link
Author

@mathetake Looking forward to your reply very much

@ncruces
Copy link
Collaborator

ncruces commented Jul 30, 2023

Can you provide us with a MCVE? Ideally a repo we can checkout and commands to run.

This is how we run wazero ourselves, and we don't see the issue, so we need to figure out (not guess) what's different.

@panchen66
Copy link
Author

panchen66 commented Jul 31, 2023

Can you provide us with a MCVE? Ideally a repo we can checkout and commands to run.

This is how we run wazero ourselves, and we don't see the issue, so we need to figure out (not guess) what's different.

this is MCVE of mock binary.decodeCode cause memory leak
Implemented in /wazero/examples/allocation/rust/greet.go

package main

import (
	"context"
	_ "embed"
	"errors"
	"fmt"
	"log"
	"net/http"
	_ "net/http/pprof"
	"os"
	"runtime"

	"github.com/tetratelabs/wazero"
	"github.com/tetratelabs/wazero/api"
)

// greetWasm was compiled using `zig build`
//
//go:embed testdata/greet.wasm
var greetWasm []byte

// main shows how to interact with a WebAssembly function that was compiled from Zig.
//
// See README.md for a full description.
func main() {
	runtime.GOMAXPROCS(1)
	runtime.SetMutexProfileFraction(1)
	runtime.SetBlockProfileRate(1)

	go func() {
		if err := http.ListenAndServe(":6060", nil); err != nil {
			log.Fatal(err)
		}
		os.Exit(0)
	}()

	for {
		ctx, mod, _ := run()
		mod.Close(ctx)
	}
}

func run() (context.Context, api.Module, error) {
	// Choose the context to use for function calls.
	ctx := context.Background()

	// Create a new WebAssembly Runtime.
	r := wazero.NewRuntime(ctx)
	defer r.Close(ctx) // This closes everything this Runtime created.

	// Instantiate a Go-defined module named "env" that exports a function to
	// log to the console.
	_, err := r.NewHostModuleBuilder("env").
		NewFunctionBuilder().WithFunc(logString).Export("log").
		Instantiate(ctx)
	if err != nil {
		return nil, nil, err
	}

	// Instantiate a WebAssembly module that imports the "log" function defined
	// in "env" and exports "memory" and functions we'll use in this example.
	mod, err := r.InstantiateWithConfig(ctx, greetWasm,
		wazero.NewModuleConfig().WithStdout(os.Stdout).WithStderr(os.Stderr))
	if err != nil {
		return nil, nil, err
	}

	// Get references to WebAssembly functions we'll use in this example.
	greet := mod.ExportedFunction("greet")
	greeting := mod.ExportedFunction("greeting")

	malloc := mod.ExportedFunction("malloc")
	free := mod.ExportedFunction("free")

	// Let's use the argument to this main function in Wasm.
	name := "test"
	nameSize := uint64(len(name))

	// Instead of an arbitrary memory offset, use Zig's allocator. Notice
	// there is nothing string-specific in this allocation function. The same
	// function could be used to pass binary serialized data to Wasm.
	results, err := malloc.Call(ctx, nameSize)
	if err != nil {
		return nil, nil, err
	}
	namePtr := results[0]
	if namePtr == 0 {
		return nil, nil, errors.New("malloc failed")
	}
	// We have to free this pointer when finished.
	defer free.Call(ctx, namePtr, nameSize)

	// The pointer is a linear memory offset, which is where we write the name.
	if !mod.Memory().Write(uint32(namePtr), []byte(name)) {
		return nil, nil, fmt.Errorf("Memory.Write(%d, %d) out of range of memory size %d",
			namePtr, nameSize, mod.Memory().Size())
	}

	// Now, we can call "greet", which reads the string we wrote to memory!
	_, err = greet.Call(ctx, namePtr, nameSize)
	if err != nil {
		return nil, nil, err
	}

	// Finally, we get the greeting message "greet" printed. This shows how to
	// read-back something allocated by Zig.
	ptrSize, err := greeting.Call(ctx, namePtr, nameSize)
	if err != nil {
		return nil, nil, err
	}

	greetingPtr := uint32(ptrSize[0] >> 32)
	greetingSize := uint32(ptrSize[0])
	// The pointer is a linear memory offset, which is where we write the name.
	if bytes, ok := mod.Memory().Read(greetingPtr, greetingSize); !ok {
		return nil, nil, fmt.Errorf("Memory.Read(%d, %d) out of range of memory size %d",
			greetingPtr, greetingSize, mod.Memory().Size())
	} else {
		fmt.Println("go >>", string(bytes))
	}

	return nil, mod, nil
}

func logString(_ context.Context, m api.Module, offset, byteCount uint32) {
	buf, ok := m.Memory().Read(offset, byteCount)
	if !ok {
		log.Panicf("Memory.Read(%d, %d) out of range", offset, byteCount)
	}
	fmt.Println(string(buf))
}

can see at http://localhost:6060/debug/pprof/heap and top

github.com/tetratelabs/wazero/internal/wasm/binary.decodeCode‘s memory is constantly growing

@panchen66
Copy link
Author

I hope to receive your reply. Thank you very much
@ncruces @mathetake

@mathetake
Copy link
Member

Now we have supposedly a repro so reopening it

@mathetake mathetake reopened this Aug 1, 2023
@panchen66
Copy link
Author

Now we have supposedly a repro so reopening it

Thank you very much for your attention

ncruces added a commit that referenced this issue Aug 1, 2023
ncruces added a commit that referenced this issue Aug 1, 2023
Fixes #1600.

Signed-off-by: Nuno Cruces <[email protected]>
@ncruces
Copy link
Collaborator

ncruces commented Aug 1, 2023

Confirmed the leak, actually it's sufficient to change:

func main() {
if err := run(); err != nil {
log.Panicln(err)
}
}

To (add infinite loop):

func main() {
	for {
		if err := run(); err != nil {
			log.Panicln(err)
		}
	}
}

And run the sample.

@ncruces ncruces added bug Something isn't working and removed question Further information is requested labels Aug 2, 2023
@panchen66
Copy link
Author

Thank you for your contributions. When will a new release be produced @mathetake

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 2, 2023

@panchen66 there are some work in progress still in the PR queue. Typically, we release once per month, but have more often sometimes. Right now, we have code already landed that has changed enough to be 1.4.0 (vs a patch). I think the following should happen before 1.4.0 (my opinion)

  1. complete and verify wasi: use File.Poll for all blocking FDs in poll_oneoff #1606
  2. at least one project convert to experimentalsys.FS Exposes writeable filesystem as experimentalsys.FS #1605
  3. Wait for Go 1.21 to be released (and remove our 1.18 support from the build)

Meanwhile, if you need to use this patch, depend on the main branch.

@evacchi
Copy link
Contributor

evacchi commented Aug 9, 2023

this is now in v1.4.0 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants