rustls / rustls/rustls-ffi

usage of Arc<T> where T is not Send or Sync

Open
#333 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
170
Forks
43
Avg merge
1d 1h
Merged PRs (30d)
10

Description

Currently cargo clippy produces this error:

$ cargo clippy 
    Checking rustls-ffi v0.10.0 (/home/jsha/rust/rustls-ffi)
error: usage of `Arc<T>` where `T` is not `Send` or `Sync`
   --> src/lib.rs:396:23
    |
396 |         Arc::into_raw(Arc::new(src)) as *const _
    |                       ^^^^^^^^^^^^^
    |
    = help: consider using `Rc<T>` instead or wrapping `T` in a std::sync type like `Mutex<T>`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
    = note: `#[deny(clippy::arc_with_non_send_sync)]` on by default

The relevant code is:

pub(crate) trait CastConstPtr {
    type RustType;

    fn cast_const_ptr(ptr: *const Self) -> *const Self::RustType {
        ptr as *const _
    }
}

pub(crate) trait CastConstPtr {
    type RustType;

    fn cast_const_ptr(ptr: *const Self) -> *const Self::RustType {
        ptr as *const _
    }
}

/// Anything that qualifies for CastPtr also automatically qualifies for
/// CastConstPtr. Splitting out CastPtr vs CastConstPtr allows us to ensure
/// that Arcs are never cast to a mutable pointer.
impl<T, R> CastConstPtr for T
where
    T: CastPtr<RustType = R>,
    R: Send,
{
    type RustType = R;
}

pub(crate) trait ArcCastPtr: CastConstPtr + Sized + Send + Sync {
    ...
    fn to_const_ptr(src: Self::RustType) -> *const Self {
        Arc::into_raw(Arc::new(src)) as *const _
    }

I think the clippy finding is a good one. Somewhere along the line we should probably be guaranteeing that the RustType for an ArcCastPtr is at least Send. I tried a couple of quick stabs at expressing this in the type system but haven't yet succeeded, so I'm filing this issue to track it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the CastConstPtr and ArcCastPtr definitions in src/lib.rs, especially the to_const_ptr code around line 396, and run cargo clippy to reproduce the warning. Determine how the relevant RustType bound should be expressed; done means the intended ArcCastPtr types satisfy the required Send or Sync guarantees and cargo clippy passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
security
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.