Stream: git-wasmtime

Topic: wasmtime / PR #2017 Fix a memory leak with command modules

Wasmtime GitHub notifications bot (Jul 14 2020 at 14:20):

alexcrichton opened PR #2017 from fix-leak to main:

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 with Weak<StoreInner>, which needs
to be plumbed through for all Extern items as well.

Wasmtime GitHub notifications bot (Jul 17 2020 at 18:39):

alexcrichton requested sunfishcode for a review on PR #2017.

Wasmtime GitHub notifications bot (Jul 20 2020 at 15:49):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Jul 20 2020 at 15:49):

sunfishcode created PR Review 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?

Wasmtime GitHub notifications bot (Jul 20 2020 at 20:53):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Jul 20 2020 at 20:53):

alexcrichton created PR Review 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.

Wasmtime GitHub notifications bot (Feb 01 2021 at 15:29):

alexcrichton updated PR #2017 from fix-leak to main:

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 with Weak<StoreInner>, which needs
to be plumbed through for all Extern items as well.

Wasmtime GitHub notifications bot (Feb 01 2021 at 15:30):

alexcrichton edited PR #2017 from fix-leak to main:

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).

Wasmtime GitHub notifications bot (Feb 01 2021 at 18:45):

peterhuene submitted PR Review.

Wasmtime GitHub notifications bot (Feb 01 2021 at 19:04):

alexcrichton merged PR #2017.

Last updated: Dec 23 2024 at 12:05 UTC