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

Slabs and handles #1808

Closed
wants to merge 6 commits into from
Closed

Slabs and handles #1808

wants to merge 6 commits into from

Commits on Oct 27, 2023

  1. Implemented slab storage that allocates handles (mozilla#1730)

    This will be used for passing handles across the FFI.  This have several
    advantages as an FFI type:
    
    * Generation counter to detect use-after-free bugs
    * Slab ID to detect using a handle with the wrong type.
    * The same data structures can be used on the foreign side, rather than us
      having to figure out how to leak references in all languages.
    * Integers come with less gotchas.  For example, we use a bit to
      differentiate between foreign and Rust handles.  This would be
      possible with tagged pointers but there's a lot of details to worry
      about there.  See the `tagged_pointer` crate.
    * Constant width at 64 bits rather than using the platform word size.
      This will simplify some things, especially reading/writing them to
      `RustBuffer`
    * Only the first 48 bits are significant which helps with languages like JS.
    
    Performance should be pretty good.  `get` and `inc_ref` are basically
    lock-free thanks to the `append_only_vec` crate and some atomic code.
    `insert` and `remove` that frees an entry use a shared lock. We could
    speed that up by keeping thread-local free lists, but that seems like
    overkill since the lock should rarely be contended.
    
    * For objects, this represents a small overhead over simply leaking the
      Arc.  The same is true for the Swift objects that we leak using
      `Unmanaged<>`.
    * For trait interfaces, this is probably a small gain compared to adding
      an extra box, then leaking it.
    * This is going to be way faster than the foreign code that uses a lock
      and a map.
    
    As mentioned above, I'm pretty sure that we can leverage this for
    foreign handles as well, and should be able to remove some code on from
    the bindings.
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    8914710 View commit details
    Browse the repository at this point in the history
  2. Use Handle for passing Rust objects across the FFI

    * Added the `SlabAlloc` FFI trait.  This is used to manage handles for
      `Arc<T>` instances, including `Arc<dyn Trait>`
    * Also use handles for trait interfaces.  This still needs some
      updates on the foreign side before it's working.
    * Renamed a bunch of stuff and replaced a lot of comment text
    * Added the `const_random` crate to generate random slab IDs.  This
      should be a pretty lightweight dpeendency.
    * Bumped `CONTRACT_VERSION` since this is a change to the FFI
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    a4fb942 View commit details
    Browse the repository at this point in the history
  3. Prevent potential use-after-free when lowering objects (mozilla#1797)

    Changed the FFI for lowering objects avoid the scenario layed out in the
    issue.  The new system is that the foreign code calls `inc_ref` on the
    handle, then the Rust code calls `remove` on it.  This effectively
    makes it so `lower` returns an owned handle instead of a borrowed one.
    
    This replaces a lot of Kotlin code that was concerned about the same
    issue in a similar situation.  Removed the `test_handlerace.kts` test
    for this, I believe this is now all handled at a lower level and we
    don't need this test.  FWIW, the test was failing, but just because we
    now raise a different exception -- and the new exception mentions that you
    may be using the handle after it's free.
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    f7b2eb5 View commit details
    Browse the repository at this point in the history
  4. Use Handle for passing foreign objects across the FFI

    Exported Rust FFI functions to manage a Slab that stores unit values.
    The foreign code uses this to allocate and manage handles then stores the
    actual data inside a native array.
    
    This replaces the previous handle maps / pointer leaking code for
    callbacks, trait interfaces and continuation data.
    
    Started updating the ForeignExecutor code to use handles, but this is
    still a WIP while the ForeignExecutor type is in it's limbo state.
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    0ea8f5f View commit details
    Browse the repository at this point in the history
  5. Use handles for better trait interfaces passing

    Check if the trait interface handle originates from the other side of
    the FFI.  If it does, inc-ref it and return the handle, rather than
    returning a handle to the wrapped object.  Before, each time the trait
    object was passed across the FFI, we wrapped it another time
    
    On the foreign side, this can be accomplished by a type check.  On the
    Rust side, this requires an extra, `#[doc(hidden)]` method on the trait.
    This means that UDL-defined trait interfaces need to be wrapped with an
    attribute macro that inserts that method.
    
    One issue with this is that it can cause us to leak references if we do
    the inc-ref, then there's an exception lowering another argument.  I
    believe this can be fixed by separating out the failable lowering code
    from the non-failable code.
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    4ab3d05 View commit details
    Browse the repository at this point in the history
  6. Updated lower validation for Python/Ruby

    The dynamic languages performed various checks in lower(): type checks, bounds
    checks, etc.  This presented an issue when we wanted to call `inc_ref`
    for object types.  If we did this, then another value failed its check,
    we would leak the reference.
    
    Prevent this by adding a `check()` method.  We do the check first then
    once we get to `lower()`, nothing can fail.
    bendk committed Oct 27, 2023
    Configuration menu
    Copy the full SHA
    d305f7e View commit details
    Browse the repository at this point in the history