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

[draft] show how Arc<Mmap> would work #9687

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sunshowers
Copy link
Contributor

Draft to show how #9682 would work. (Will clean this up before putting up.)

In upcoming work we're going to centralize memory management inside `Mmap`.  In
order to do that, we need memory that logically borrows from the `Mmap` to have
access to it. That turns out to be possible in some situations but very
difficult in others (see bytecodealliance#9681 for an attempt that was a real hassle to get
working).

To prepare for this work, start moving these uses to be behind `Arc`. These
aren't cloned at the moment, but they will be in the future.
Return a single `SendSyncPtr` -- the platform-independent context converts to
the various raw pointer types as desired. This simplifies upcoming work where I
wanted to return a `SendSyncPtr`.
This is part of the work to centralize memory management into the `mmap`
module. This commit introduces a few structures which aid in that process, and
starts converting one of the functions (`MemoryImageSource::map_at`) into this
module.

The structures introduced are:

* `MemoryBase`: `RuntimeLinearMemory::base_ptr` is now
  `RuntimeLinearMemory::base`, which returns a `MemoryBase`. This is either a
  raw pointer or an mmap + an offset into it.

* `MmapOffset`: A combination of a reference to an mmap and an offset into it.
  Logically represents a pointer into a mapped section of memory.

* `MmapOffsetRaw`: Some components like `MemoryImageSlot` logically work on
  borrowed memory, but adding lifetime parameters to them would introduce
  self-reference issues. Instead, store a raw form of the `MmapOffset` such
  that it can be reconstructed at runtime. This should work for most future
  work here, but not all of it -- I've written out some comments along with
  ideas.
Comment on lines +540 to +546
// Note that this code would be incorrect if clear-on-drop
// were enabled. That's because in the struct definition,
// `memory_image` above is listed after `alloc`. Rust drops
// fields in the order they're defined, so `memory_image`
// would be dropped after `alloc`. If clear-on-drop were
// enabled, `memory_image` would end up remapping a bunch of
// memory after it was unmapped by the `alloc` drop.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a subtle dependency that took me a bit to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And this is only dangerous when alloc represents an owned memory allocation. It's fine if alloc is borrowed. This is a subtle point.)

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant