Fix a memory leak with command modules #2017
Conversation
github-actions
bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
crates/wasmtime/src/linker.rs
Outdated
| let imports = imports | ||
| .iter() | ||
| .map(|i| i.upgrade().unwrap()) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
It's not immediately clear to me how this fixes the cycle. Func also contains a strong StoreInstanceHandle and Instance contains a strong Store, so is there already a cycle?
There was a problem hiding this comment.
The problem here is that a strong reference to Store was closed over in the Func, which meant that Store basically had a strong reference to itself. By only closing over weak references that reference is broken at least. Each of the captures here only capture a weak version of the Store or external item.
This commit fixes a memory leak that can happen with `Linker::module` when the provided module is a command. This function creates a closure but the closure closed over a strong reference to `Store` (and transitively through any imports provided). Unfortunately a `Store` keeps everything alive, including `Func`, so this meant that `Store` was inserted into a cycle which caused the leak. The cycle here is manually broken by closing over the raw value of each external value rather than the external value itself (which has a strong reference to `Store`).
alexcrichton
force-pushed
the
fix-leak
branch
from
68f23a1 to
93a7e81
Compare
|
@sunfishcode mind taking another look at this? |
This commit fixes a memory leak that can happen with
Linker::modulewhen the provided module is a command. This function creates a closure
but the closure closed over a strong reference to
Store(andtransitively through any imports provided). Unfortunately a
Storekeeps everything alive, including
Func, so this meant thatStorewasinserted into a cycle which caused the leak.
The cycle here is manually broken by closing over the raw value of each
external value rather than the external value itself (which has a
strong reference to
Store).