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

Track more safety invariants in the type system #279

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kixunil
Copy link

@Kixunil Kixunil commented Feb 10, 2021

Meta - about this PR

This is a byproduct of attempting to figure out #278 by "asking the compiler if it sees a problem".
The idea seemed to be possibly useful, so this should be viewed almost like a draft/suggestion.

Description

This change creates several wrappers around unsafe operations to
better track the safety invariants. While not perfect, which should
reduce the number of footguns in the code.

It focuses on locking - using primitives like mutex guards known from
std. The locked state is tracked in those guards. This crate needs
some special features however. It needs to lock all locks in a slice and
unlock them in the same order. Similarly it needs to lock a queue inside
a Bucket while keeping reference to the bucket.

Additional specialized wrappers maintain safety of these advanced
operations similarly to guards. They are implemented in their own
modules to avoid access from other parts of code.

This change comes with two more side effects:

  • Cell was removed from the bucket as now locking provides required
    safety.
  • Bucket pair uses Option to represent same buckets. This adds a
    slight performance penalty but it's in slow path and may be optimized
    out by the compiler. It's even possible that absence of Cell will
    enable even more optimizations.

This change creates several wrappers around `unsafe` operations to
better track the safety invariants. While not perfect, which should
reduce the number of footguns in the code.

It focuses on locking - using primitives like mutex guards known from
`std`. The locked state is tracked in those guards. This crate needs
some special features however. It needs to lock all locks in a slice and
unlock them in the same order. Similarly it needs to lock a queue inside
a `Bucket` while keeping reference to the bucket.

Additional specialized wrappers maintain safety of these advanced
operations similarly to guards. They are implemented in their own
modules to avoid access from other parts of code.

This change comes with two more side effects:

* `Cell` was removed from the bucket as now locking provides required
  safety.
* Bucket pair uses `Option` to represent same buckets. This adds a
  slight performance penalty but it's in slow path and may be optimized
  out by the compiler. It's even possible that absence of `Cell` will
  enable even more optimizations.
@Kixunil Kixunil mentioned this pull request Feb 10, 2021
@Kixunil
Copy link
Author

Kixunil commented Feb 10, 2021

Ah, this has lower MSRV. I'm too exhausted to fix it now. Will come back later.

Some more ideas to consider:

  • Implement queue operations as methods on Queue
  • Add BucketPair wrapper type holding the pair, providing a nice getter and ensuring correct drop order. (unrelated to Possible bug - UB #278 but nice to have)

`Cell::from_mut()` was stabilized in 1.37 but the MSRV of this crate is
1.36.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant