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

Safety #765

Merged
merged 47 commits into from
Sep 24, 2023
Merged

Safety #765

merged 47 commits into from
Sep 24, 2023

Commits on Jul 23, 2023

  1. Switch UnprotectedStorage::get_mut to accept &self

    This is the first step for addressing a soundness issue where parallel joins
    create aliasing mutable references to the storage and where in regular joins for
    some storages previously returned references will be invalidated by calling
    `get_mut` (at least under stacked borrows afaict).
    
    The internals of each storage are adjusted to store components within a
    `SyncUnsafeCell` to allow handing out mutable references. `SyncUnsafeCell` is a
    wrapper of `UnsafeCell` that provides `Sync` by default.
    
    Other various details:
    * Edge cases with `as` casts on 16-bit and 32-bit platforms addressed to avoid
      UB in `UnprotectedStorage` impls.
    * Safety documentation added to unsafe code usage within `UnprotectedStorage` impls.
    * Safety documentation added to unsafe impls of DistinctStorage (only in
      storages.rs)
    * Started introduction of `#[deny(unsafe_op_in_unsafe_fn)]` lint in various
      modules.
    * Safety requirements on `UnprotectedStorage::get/get_mut` updated.
    * `NullStorage` internals updated to handle ZSTs better (including properly
      dropping them when `clean` is called and not dropping them in `insert`) and
      not require `T: Default`.
    * In `Storage::insert` add the `id` to the mask after calling `inner.insert()`
      to protect against unwinding from the `insert` call.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    2f331ad View commit details
    Browse the repository at this point in the history
  2. Update safety comments on uses of UnprotectedStorage::get_mut and c…

    …hange how
    
    `UnprotectedStorage::clean` works.
    
    * `clean` now always clears all components even if dropping the storage would
      have dropped them automatically (this helps address an edge case with
      `DenseVecStorage::insert` overflowing a `u32`).
    * Add safety requirement to `clean` that indicates the caller should ensure
      the mask has been cleared even if unwinding occurs from the `clean` call.
    * Ensured uses of `clean` met this requirement.
    * Also continue expanding application of `unsafe_op_in_unsafe_fn` lint.
    * Fixed typo from previous commit where FlaggedStorage::get was using get_mut
      internally.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    8366ddb View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    ba8b976 View commit details
    Browse the repository at this point in the history
  4. Update DerefFlaggedStorage to account for changes, get_mut impl postp…

    …oned since
    
    we most likely want to transition this to a streaming only storage (which is
    thus given &mut access).
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    ac2146d View commit details
    Browse the repository at this point in the history
  5. Refactor UnprotectedStorage and add SharedGetAccessMut trait

    Implementations and uses of these traits are not yet changed. However, hopefully
    this is the final form needed to have storages that only support
    lending/streaming joins while also allowing some storages to support regular
    `Iterator` like joins and parallel joins as well as allowing storages where
    `UnprotectedStorage<T>::AccessMut` doesn't implement `DerefMut<Target =T>` (like
    a planned variant of the flagged storage).
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    bf620e1 View commit details
    Browse the repository at this point in the history
  6. Refactor Join family of traits so they can be more safely implement…

    …ed/used:
    
    (NOTE: nothing compiling yet since implementation of these traits have not been
    updated)
    
    * Introduced `LendJoin` trait that is like the lending iterator version of
      `Join`. This is useful for types that need to return aliasing mutable
      references from `get` calls with distinct `id`s (e.g. `Entries`,
      `DerefFlaggedStorage`, `RestrictStoraged`). `LendJoin` uses `nougat` crate to
      provide a GAT based API on stable rust.
    * Removed unsound `JoinIter::get`/`JoinIter::get_unchecked` but these methods
      are present on `JoinLendIter` where they can be soundly implemented.
    * Since there is a single `MaybeJoin` type used for all joins, the convenient
      `.maybe()` method was moved to `LendJoin` which should be the common
      denominator of implemented join traits (if we put this method on multiple
      traits, rust might start wondering which one you want to call, which isn't
      convenient...).
    * `ParJoin` trait is no an longer empty trait that relies on the implementation
      in `Join`. The new `ParJoin::get` takes a shared reference so the
      `ParallelIterator` implementation no longer creates aliasing exclusive
      references to call `Join::get`.
    * `Join` is now an `unsafe` trait to require that the mask/values returned from
      `Join::open` are properly associated.
    * Extended application of `deny(unsafe_op_in_unsafe_fn)` to the `join` module
      and added safety documentation to calls to unsafe functions there.
    * Removed `Clone` implementation for `JoinIter<J> where J::Mask: Clone,
      J::Value: Clone`. Nothing, in `Join::get` safety requirements implies that
      this is safe, in the cases where this is safe, the user can just call
      `.join()` twice for similar effect.
    
    Other misc changes:
    
    * `BitAnd` helper trait and `MaybeJoin` struct moved to their own files to
      declutter `join/mod.rs`.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    e7d0aa1 View commit details
    Browse the repository at this point in the history
  7. Update Join/ParJoin/Unprotected implementations to match changes in t…

    …hese traits
    
    and add LendJoin implementations.
    
    Compiles again!!!
    
    * Several Join implementors where commented out (marked with `D-TODO`) so that I
      can update them in a separate batch. Want to make sure the changes were
      working first.
    * Remove `where Self: 'next'` bound from `LendJoin::Type<'next>'` since this was
      causing issues and an unnecessary bound.
    * Fix several other errors related to usage of `LendJoin`'s GAT.
    * Fix other misc errors from the last few commits
    * `deny(unsafe_op_in_unsafe_fn)` now covers the whole crate.
    * Add safety comments to unsafe code used in `Generation` methods.
    
    Still need to:
    * Implement `SharedGetAccessMutStorage` for relevant storages.
    * Update commented out types that implement `Join`.
    * Update some safety comments.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    cc97588 View commit details
    Browse the repository at this point in the history
  8. Implement SharedGetAccessMutStorage for applicable storages and updat…

    …e storage
    
    safety comments.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    483914b View commit details
    Browse the repository at this point in the history
  9. Remove get_mut, rename get_access_mut -> get_mut, rename

    `SharedGetAccessMutStorage` -> `SharedGetMutStorage`, rename
    `shared_get_access_mut` -> `shared_get_mut`.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    57e929c View commit details
    Browse the repository at this point in the history
  10. Various next steps:

    * Start work on implementing LendJoin and safely re-implementing Join for
      `&ChangeSet`, `&mut ChangeSet`, and `ChangeSet`.
    * Add `AccessMut` trait as a replacement for a few cases that were using
      `DerefMut` (since we don't want to require that
      `UnprotectedStorage::AccessMut<'a>' has to implement `DerefMut`). IIRC the
      cases were originally missed because they are behind feature flags.
    * Modify `SharedGetMutOnly` to also be generic over the storage type so that we
      don't have to require `T: Component` (since we were getting the storage type
      from the associated `Component::Storage`). IIRC this is to support use in
      `ChangeSet<T>` which doesn't require `T: Component`.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    0968089 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    25b27e5 View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    448c9de View commit details
    Browse the repository at this point in the history
  13. Changes to allow soundly implementing Join/LendJoin for the owned Cha…

    …ngeSet<T>
    
    where iterating it removes items.
    
    * Added additional requirement to Join::get/LendJoin::get that it can not be
      called multiple times with the same ID.
    * Added unsafe `RepeatableLendGet` trait to allow opt-out of this requirement so
      that a safe `JoinLendIter::get` method can remain exposed.
    * Updated relevant safety comments for uses/impls of `LendJoin::get`.
    * TODO for next commit: update all uses/impls of `Join::get` to ensure they
      correspond with the requirement changes.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    919434b View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    d2d05f2 View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    ce90ce7 View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    bea1ea9 View commit details
    Browse the repository at this point in the history
  17. Remove "nightly" feature now that generic associated types have stabi…

    …lized and
    
    bump the MSRV to 1.65.0
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    e22d22a View commit details
    Browse the repository at this point in the history
  18. Uncomment entry module and rework implementation:

    * Replace `Join` impl with `LendJoin` (to avoid creating aliasing mutable
      references to the storage).
    * Create new `Storage::not_present_insert` method that requires that the `id`
      not be present in the mask. This is used by both `Storage::insert` and
      `VacantEntry::insert` so we can centralize documenting the safety of calling
      `UnprotectedStorage::insert` and the handling of potential unwinding from
      `BitSet::add`.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    738dc16 View commit details
    Browse the repository at this point in the history
  19. Implement refactored Join traits for RestrictedStorage and other re…

    …lated
    
    changes:
    
    * SharedGetMutOnly::get_mut changed from method to associated function to make
      its use more apparent (e.g. compared to calling UnprotectedStorage::get_mut).
    * New requirement added to ParJoin trait implementation to facilitate
      callers of ParJoin::get that need to ensure they don't repeat indices.
    * `ShareGetMutStorage::shared_get_mut` requirements tweaked to allow calling
      this in conjuction with `UnprotectedStorage::get` when the `id`s used don't
      overlap. This facilitates `Join`/`ParJoin` impls for `RestrictedStorage` which
      can allow getting a component for one entity mutably while immutably getting
      the component for another entity at the same time.
    * Marker types used for restricted storage implementation replaced with
      producing distinct types for different types of joins: `PairedStorageRead`
      (for any read only join), `PariedStorageWriteExclusive` (for mutable
      LendJoin), and `PairedStorageWriteShare` (for mutable Join/ParJoin).
    * Renamed `PairedStorage` (which was replaced with the 3 types above) methods
      `get_unchecked`/`get_unchecked_mut` to `get`/`get_mut` since `unchecked` often
      is used to indicate some safety requirement hasn't been checked which isn't
      the case here. Renamed existing `get`/`get_mut` methods to
      `get_other`/`get_mut_other`.
    * Other misc changes that were missed in previous commits.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    e4c3655 View commit details
    Browse the repository at this point in the history
  20. Update implementations of ParJoin and callers of ParJoin::get to refl…

    …ect changes
    
    in safety requirements.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    6ca2338 View commit details
    Browse the repository at this point in the history
  21. Configuration menu
    Copy the full SHA
    0fad41d View commit details
    Browse the repository at this point in the history
  22. Configuration menu
    Copy the full SHA
    2c7d5e6 View commit details
    Browse the repository at this point in the history
  23. Configuration menu
    Copy the full SHA
    c33cd7c View commit details
    Browse the repository at this point in the history
  24. Configuration menu
    Copy the full SHA
    09f1a4c View commit details
    Browse the repository at this point in the history
  25. Reduce loop iterations on some tests when running with Miri so that t…

    …hey finish in a reasonable time
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    c3a301f View commit details
    Browse the repository at this point in the history
  26. Appease clippy

    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    e8f80e7 View commit details
    Browse the repository at this point in the history
  27. Configuration menu
    Copy the full SHA
    391b310 View commit details
    Browse the repository at this point in the history
  28. Enhance LendJoin docs to hopefully explain its purpose and usage

    Also:
    * Make `lend_join` example more comprehensive to showcase the options for
      iterating without the Iterator trait, as well as the JoinLendIter::get method.
      Include comments in the example to explain different aspects.
    * Fix/add various links in code docs.
    * Publically export `SliceAccess` trait since it appears in bounds on the public
      `Storage::as_slice`/`Storage::as_mut_slice`.
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    6b74393 View commit details
    Browse the repository at this point in the history
  29. Configuration menu
    Copy the full SHA
    5aa5abf View commit details
    Browse the repository at this point in the history
  30. Configuration menu
    Copy the full SHA
    2beb089 View commit details
    Browse the repository at this point in the history
  31. Configuration menu
    Copy the full SHA
    cbfa283 View commit details
    Browse the repository at this point in the history
  32. Tweak rustfmt config

    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    819d6ca View commit details
    Browse the repository at this point in the history
  33. fmt

    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    2400172 View commit details
    Browse the repository at this point in the history
  34. Fix and simply insert code unwinding handling since allocation is not…

    … actually guaranteed to abort on failure
    Imberflur committed Jul 23, 2023
    Configuration menu
    Copy the full SHA
    6736f15 View commit details
    Browse the repository at this point in the history
  35. Configuration menu
    Copy the full SHA
    a31e781 View commit details
    Browse the repository at this point in the history

Commits on Jul 24, 2023

  1. Fix typo in docs

    Imberflur committed Jul 24, 2023
    Configuration menu
    Copy the full SHA
    1fc3d1f View commit details
    Browse the repository at this point in the history

Commits on Jul 25, 2023

  1. Fix various compilation warnings, change abort on unwinding during in…

    …sertion into removing the inserted component (mainly to make should_panic test work)
    Imberflur committed Jul 25, 2023
    Configuration menu
    Copy the full SHA
    5abd46a View commit details
    Browse the repository at this point in the history

Commits on Jul 26, 2023

  1. Configuration menu
    Copy the full SHA
    de30c85 View commit details
    Browse the repository at this point in the history
  2. Update MSRV to 1.70 since that is apparently necessary to remove "whe…

    …re Self: 'next" from LendJoin::get
    Imberflur committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    35da879 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    a8c96f8 View commit details
    Browse the repository at this point in the history

Commits on Sep 15, 2023

  1. Configuration menu
    Copy the full SHA
    784c7b0 View commit details
    Browse the repository at this point in the history

Commits on Sep 16, 2023

  1. Configuration menu
    Copy the full SHA
    5d3bfe7 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    a53d28e View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    0a4b99a View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    6d4724b View commit details
    Browse the repository at this point in the history
  5. Update changelog

    Imberflur committed Sep 16, 2023
    Configuration menu
    Copy the full SHA
    eecf83a View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    276450e View commit details
    Browse the repository at this point in the history