Stream: wasmtime

Topic: Freeing resources in go bindings


view this post on Zulip Geoff Goodman (Feb 15 2024 at 17:08):

I tried doing a simple POC with the golang wasmtime bindings. The idea was that I would isolate a workload on a per-request basis. To accomplish this, I had a single engine and was creating per-request stores, linkers and instances. The instances were based on a pre-compiled WASM module.

I noticed that I quickly OOM'd. Also didn't see obvious ways to explicitly free up those ephemeral objects.

My guess is that I'm probably holding it wrong but wanted to know if others have seen such issues and how they got around them.

view this post on Zulip Joel Dice (Feb 15 2024 at 17:29):

We do per-request isolation in Spin using InstancePre to make a new Instance (with a new Store) for each request with a variety of languages (including (Tiny)Go) and haven't had such issues. Can you verify that you're definitely dropping the instance and store after each request?

view this post on Zulip Geoff Goodman (Feb 15 2024 at 17:51):

@Joel Dice I'm embedding wasmtime in a go project, not running go as a guest language. I'm NOT explicitly dropping the instance and store because I couldn't find any way to do so.

view this post on Zulip Geoff Goodman (Feb 15 2024 at 17:53):

It might be a gap in the bindings or just some trickery with Contexts that I didn't figure out.

view this post on Zulip Joel Dice (Feb 15 2024 at 17:54):

Ah, I see. Yeah, I don't have experience with wasmtime-go myself. I'm guessing it's based on Wasmtime's C bindings, which presumably has a way to dispose of such resources. Might be worth looking through the wasmtime-go source code and/or opening an issue on that repo.

view this post on Zulip Geoff Goodman (Feb 15 2024 at 17:55):

Ah, let me see if I can find out the c api.

view this post on Zulip Geoff Goodman (Feb 15 2024 at 17:57):

OK, found them so it looks like the c bindings don't expose an explicit drop() or free() method. It seems like they expect a finalizer. Is that like a destructor?

https://github.com/bytecodealliance/wasmtime/blob/83a5a1a607f5609249e451cbadd38827140bb7eb/crates/c-api/src/store.rs#L88-L93

view this post on Zulip Lann Martin (Feb 15 2024 at 17:58):

A finalizer is a Go runtime feature that frees resources once a particular object gets garbage collected

view this post on Zulip Lann Martin (Feb 15 2024 at 17:59):

So you'd need to remove any references to the store, including any "child" objects that might refer to it

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:02):

In my little POC, the only long-lived references are these:

    engine := wasmtime.NewEngine()

    mod, err := wasmtime.NewModule(engine, quickjsWasmBytes)
    if err != nil {
        slog.Error("Error instantiating module: %e", err)
        os.Exit(1)
    }

Beyond that, everything is assigned in the request handler and _should_ only have 1-way references back to the shared engine.

Perhaps I'm making a mistake w/ per-request linkers?

view this post on Zulip Joel Dice (Feb 15 2024 at 18:02):

@Lann Martin But it's not an reference-counting GC, is it? So it won't be collected until there's Go-visible memory pressure, which C and Rust allocations won't necessarily contribute to. I think we'll want to have a pro-active way of disposing these things without leaning on the GC.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:03):

You'd hope GC would run before OOM but yeah it isn't deterministic

view this post on Zulip Lann Martin (Feb 15 2024 at 18:03):

agree there should probably be a way to explicitly deallocate

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:04):

I think the idiomatic trick in go would be:

        store := wasmtime.NewStore(engine)
        defer store.Drop()

view this post on Zulip Joel Dice (Feb 15 2024 at 18:04):

I can almost guarantee it won't run before OOM if it thinks each of these Instance and Store objects are tiny and doesn't even see the C/Rust objects they point to as being part of the heap it is managing.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:05):

@Geoff Goodman You'd maybe call it store.Close() in go, but yes

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:05):

I'm neither a rustacean nor gopher so I defer to the experts :D

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:09):

How does that passed-in finalizer work in the c bindings?

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:10):

#[no_mangle]
pub extern "C" fn wasmtime_store_new(
    engine: &wasm_engine_t,
    data: *mut c_void,
    finalizer: Option<extern "C" fn(*mut c_void)>,
) -> Box<wasmtime_store_t> {

For reference: https://github.com/bytecodealliance/wasmtime/blob/83a5a1a607f5609249e451cbadd38827140bb7eb/crates/c-api/src/store.rs#L88-L93

view this post on Zulip Lann Martin (Feb 15 2024 at 18:10):

finalizers are a feature of the Go garbage collector; they have nothing to do with wasmtime or C

view this post on Zulip Joel Dice (Feb 15 2024 at 18:11):

Looks like he's referring to a Wasmtime C API thing independent of Go, though

view this post on Zulip Lann Martin (Feb 15 2024 at 18:11):

ohh sorry; maybe an unfortunate terminology collision?

view this post on Zulip Joel Dice (Feb 15 2024 at 18:12):

Looks to me like the finalizer param is in case the data param needs to be disposed with custom code supplied by the embedder

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:13):

If someone is using the c bindings, how does one drop a store?

view this post on Zulip Lann Martin (Feb 15 2024 at 18:14):

wasmtime_store_delete: https://github.com/bytecodealliance/wasmtime-go/blob/v1.0.0/store.go#L72

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:15):

Nice. Missed that in the header. Seems like that offers a nice path to opt into explicit resource management.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:16):

It would probably require some more changes to the Go API to coexist with the Go finalizer

view this post on Zulip Joel Dice (Feb 15 2024 at 18:16):

You'll want to set store._ptr to null after you delete it (and not call wasmtime_store_delete if it's already null) so it doesn't get deleted twice in the finalizer.

view this post on Zulip Joel Dice (Feb 15 2024 at 18:17):

And check for null in all the methods.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:17):

yeah really needs to be an issue on the wasmtime-go repo for discussion

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:17):

I can kick that off.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:17):

Thanks!

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:17):

When I originally wrote the Go bindings I was hoping to lean heavily on the Go GC to clean things up to avoid the need for .Drop() everywhere since sometimes the lifetimes can be pretty tricky, but I do agree that we should add an explicit Drop method where needed!

Is it possible to get a snapshot of the heap when OOM was triggered? There may also be a bug in the wasmtime-go bindings that is leaking something by accident

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:18):

(also agreed an issue would be good to track as well)

view this post on Zulip Joel Dice (Feb 15 2024 at 18:26):

To reiterate (since this is kind of a pet peeve of mine): whenever a GC-allocated object points to resources outside the GC-managed heap (e.g. sockets, malloc'd buffers, graphics contexts, file descriptors, mmap'd regions, etc.) and the only way to clean them up is via GC-triggered finalizers, you're going to have a bad time eventually. The GC doesn't see any of those external resources as contributing to memory pressure for the GC-managed heap, so you can easily run out of those resources before the next GC cycle.

Which isn't to criticize use of finalizers to dispose of such things as a good (and ergonomic) first step, but there's a reason why e.g. Python got a with statement, Java got try-with-resources, and Go has defer -- they all acknowledge that some resources need to be cleaned up independently of GC.

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:27):

Even JavaScript is getting using and await using.

view this post on Zulip Lann Martin (Feb 15 2024 at 18:29):

fwiw, using finalizers is quite rare in Go; people expect to manually clean up unmanaged resources

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:37):

Issue created, thanks for the insight folks: https://github.com/bytecodealliance/wasmtime-go/issues/209

In a use-case that creates ephemeral stores and instances at high volume, such as on a per-request basis, it is easy to exhaust available memory. Currently, it appears that the go bindings rely on ...

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:39):

This is a bit orthogonal at this point, but are y'all saying that wasmtime-go shouldn't have any finalizers at all? I agree it should have the option to explicitly finalize, but it'd be surprising to me if it's recommended to remove all finalizers

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:40):

insofar as I'd expect a managed language like Go to not have memory leaks by default, but if finalizers were removed then the wasmtime bindings would have memory leaks by default

view this post on Zulip Lann Martin (Feb 15 2024 at 18:40):

go finalizers have pretty weak guarantees, even in practice

view this post on Zulip Joel Dice (Feb 15 2024 at 18:41):

I don't know what's idiomatic in Go, so no opinion on that from me, but you could equally say the C ABI has memory leaks by default (i.e. you have to explicitly call wasmtime_store_delete to avoid them).

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:41):

Well no one expects C to manage memory for you, but I figured that users of Python and Go for example expect to not have memory leaks by default

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:41):

e.g. if I open a file in Python I'd expect that it'd eventually get closed when I stop referring to it

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:42):

like I understand nothing is deterministic about finalizers and it's a good point that finalizers hide the "true cost" of an object, but despite all that I was always under the impression that finalizers are exactly intended for this sort of use case of C ffi bindings cleanup

view this post on Zulip Joel Dice (Feb 15 2024 at 18:42):

Python is interesting because it has a reference-counting+plus-cycle-collector GC, so it tends to be more deterministic in practice (and thus maybe lean more heavily on finalizers).

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:42):

(again not saying explicit finalizers shouldn't exist, that unambiguously is a good idea)

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:43):

I'm also asking b/c this would be a major change to the python/go bindings to remove finalizers

view this post on Zulip Joel Dice (Feb 15 2024 at 18:43):

FWIW, I think using finalizers as "a backup" for explicit disposal makes sense, and is common in Java at least. I just don't know what's right for Go.

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:43):

(I understand that the issue isn't saying remove finalizers, so talking about something somewhat orthogonal now)

view this post on Zulip Joel Dice (Feb 15 2024 at 18:44):

Another option would be to print a warning in the finalizer.

view this post on Zulip Joel Dice (Feb 15 2024 at 18:44):

Which I've seen in Python.

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:44):

yeah that makes sense to me, but it from

fwiw, using finalizers is quite rare in Go; people expect to manually clean up unmanaged resources

it sounded like @Lann Martin was saying we should remove the current finalizers

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:44):

oh interesting (about warnings)

view this post on Zulip Lann Martin (Feb 15 2024 at 18:45):

that was more a response to @Joel Dice's micro-rant :smile:

view this post on Zulip Lann Martin (Feb 15 2024 at 18:46):

I would agree that using a finalizer as backup to an explicit close makes sense

view this post on Zulip Joel Dice (Feb 15 2024 at 18:46):

We should print a warning in the finalizers that includes a link to my micro-rant on Zulip.

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:46):

can I include your email in the warning?

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:47):

thanks Geoff for opening https://github.com/bytecodealliance/wasmtime-go/issues/209! would you be interested in helping to implement that? I'd be happy to review!

It also sounds like we should mirror that in the wasmtime-py Python bindings

In a use-case that creates ephemeral stores and instances at high volume, such as on a per-request basis, it is easy to exhaust available memory. Currently, it appears that the go bindings rely on ...

view this post on Zulip Joel Dice (Feb 15 2024 at 18:49):

FWIW, on the Python guest side we make resource handles implement ContextManager, allowing them to be used in with statements. I'm guessing we'll want to do something similar on the host side.

Likewise, it might be worth looking at how the folks building guest tooling for Go are handling resource handles.

view this post on Zulip Joel Dice (Feb 15 2024 at 18:50):

resource handle lifetimes really have to be managed explicitly due to parent-child relationships

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:50):

for sure yeah

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:51):

this really shows that someone other than me, who knows very little of Python and Go, should be primarily maintaining wasmtime-{go,py}

view this post on Zulip Joel Dice (Feb 15 2024 at 18:52):

(the silence you're hearing is the sound of nobody volunteering)

view this post on Zulip Alex Crichton (Feb 15 2024 at 18:53):

it's ok issues are enough of a signal for now :)

view this post on Zulip Geoff Goodman (Feb 15 2024 at 18:53):

Sorry, in a 1:1

view this post on Zulip Geoff Goodman (Feb 15 2024 at 19:21):

I'd be interested in the work but I can't realistically commit to the undertaking without setting folks (and myself) up for disappointment. I'm still in the exploratory phase in seeing if WASM might form a strategic investment for us. I'm personally very bullish about the tech but there are quite a few more stakeholders.

view this post on Zulip Joel Dice (Feb 15 2024 at 19:24):

I didn't mean to imply that you should volunteer to be the new wasmtime-{go,py} maintainer, FWIW. What you're saying is totally reasonable.

view this post on Zulip Joel Dice (Feb 15 2024 at 19:25):

I was just teasing Alex since he's maintainer of half the projects on the Internet.

view this post on Zulip Ralph (Feb 15 2024 at 19:26):

only 40%

view this post on Zulip Guy Bedford (Feb 15 2024 at 20:03):

resource handle lifetimes really have to be managed explicitly due to parent-child relationships

FWIW in JS we got around this by having the finalizer function closure itself maintain a strong GC link to the parent resource, so the GC can still work despite these, as suggested by @Alex Crichton

view this post on Zulip Lann Martin (Feb 15 2024 at 20:17):

This seems very tricky since dropping a resource is sometimes a positive signal in e.g. wasi-http

view this post on Zulip Guy Bedford (Feb 15 2024 at 20:20):

(yeah, we support GC as the backup, with explicit resource management as the first line approach (ready for the soon to be shipped using)) I don't think there's any disagreement here just noting that GC isn't inhibited by child resource relations

view this post on Zulip Joel Dice (Feb 15 2024 at 21:11):

Guy Bedford said:

FWIW in JS we got around this by having the finalizer function closure itself maintain a strong GC link to the parent resource, so the GC can still work despite these, as suggested by Alex Crichton

That's a cool trick. I should do that in componentize-py, too.

view this post on Zulip Geoff Goodman (Feb 16 2024 at 00:49):

@Guy Bedford any chance you could point me to where you are employing that trick?

view this post on Zulip Mossaka (Joe) (Feb 20 2024 at 19:15):

this really shows that someone other than me, who knows very little of Python and Go, should be primarily maintaining wasmtime-{go,py}

I will discuss this in the sig go subgroup to check interests for maintaining the wasmtime-go sdk.

view this post on Zulip Geoff Goodman (Feb 21 2024 at 21:38):

@Alex Crichton a big thank you. Testing now.

view this post on Zulip Geoff Goodman (Feb 22 2024 at 00:48):

No more memory exhaustion AFAICT. :thumbs_up:


Last updated: Dec 23 2024 at 13:07 UTC