Skip to content

runtime: temporarily store waspi2 memory allocations from cabi_realloc #4897

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

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

ydnar
Copy link
Contributor

@ydnar ydnar commented May 17, 2025

This is an initial take at solving the interaction between the TinyGo GC and the Component Model allocation scheme (cabi_realloc).

Specifically, when the host calls an exported function with argument(s) that require allocations, these are first allocated in the guest via calls to cabi_realloc, then these pointer(s) are passed to the function exported with go:wasmexport.

Because these allocated pointers are not stored anywhere in the TinyGo stack or heap, they are subject to immediate collection. This can happen if the host wants to copy a large string into the guest memory, and performs a sequence of cabi_realloc calls to grow the target memory.

The fix is simple: hold onto the pointers allocated by cabi_realloc until the next wasmexport function returns.

This logic currently works for reactor modules, where allocations created with cabi_realloc are held in memory until a go:wasmexport function exits. Caveat: when used with a long-running program like the default wasi-cli world, any allocations created via cabi_realloc will be held forever until func main exits.

This is intended to fix the problem in bytecodealliance/go-modules#348.

@deadprogram
Copy link
Member

I think I have run into a similar problem using wasm-unknown and wasip1 having to do with lifetimes and memory. This PR makes me think about some ways to possibly look into it further for those targets.

In any case, this seems fine to me if it does fix the wasip2 issue as described.

@dgryski any comment here?

@ydnar
Copy link
Contributor Author

ydnar commented May 18, 2025

I ran into a related issue with the wasmimport functions today and have a reproducer.

Unfortunately the fix in this PR will not fix that issue, and instead result in a memory leak.

Do not merge this.

@deadprogram deadprogram marked this pull request as draft May 18, 2025 17:49
@aykevl
Copy link
Member

aykevl commented May 21, 2025

Because these allocated pointers are not stored anywhere in the TinyGo stack or heap, they are subject to immediate collection. This can happen if the host wants to copy a large string into the guest memory, and performs a sequence of cabi_realloc calls to grow the target memory.

We have the same issue for libc malloc/realloc. A simple solution would be to call libc_realloc instead (defined in arch_tinygwasm_malloc.go). That would also avoid duplicating this logic.

(I also see that libc_realloc has a bug in the precise GC: it allocates using make([]byte, ...) which tells the GC the buffer doesn't contain pointers but that is of course an incorrect assumption with C calloc).

EDIT: See: #4898

ydnar and others added 2 commits May 25, 2025 14:10
This is an initial take at solving the interaction between the TinyGo GC and the Component Model allocation scheme (cabi_realloc).
  - Do not use make([]byte, ...) to allocate, instead allocate a slice
    of pointers. This makes sure the precise GC will scan the contents
    of the allocation, since C could very well put pointers in there.
  - Simplify the map to use the pointer as the key and the size as the
    value, instead of storing the slices directly in the map.
@ydnar ydnar force-pushed the ydnar/wasmexport-realloc branch from 1889b33 to c8bb418 Compare May 25, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants