Unsound Sync implementation for Locked

[steffahn]

As documented we have:

impl<'a, T> Sync for Locked<'a, T>
where
    T: Send,

which is incorrect. It needs to be

impl<'a, T> Sync for Locked<'a, T>
where
    T: Sync,

instead.

---

Exploitation:

use std::cell::Cell;
use try_lock::{Locked, TryLock};

// run in debug mode for clearly visible UB
fn main() {
    let cell = Cell::new(Ok::<&[usize], &[()]>(&[]));
    let lock = TryLock::new(cell);
    // now this guard is `Sync`!
    let guard: Locked<'_, Cell<_>> = lock.try_lock().unwrap();
    let bad_slice = std::thread::scope(|s| {
        let handle = s.spawn(|| {
            // now we got a shared `&Cell` into another thread!
            let r: &Cell<_> = &guard;
            // racey-race…
            loop {
                let s = r.get();
                if let Ok(v) = s {
                    if !v.is_empty() {
                        break v;
                    }
                }
            }
        });
        let r: &Cell<_> = &guard;
        // racey-race…
        loop {
            r.set(Ok(&[]));
            r.set(Err(&[(); usize::MAX]));
            if handle.is_finished() {
                break;
            }
        }
        handle.join().unwrap()
    });
    println!("Here’s all your memory :-)\n{bad_slice:?}");
}

[seanmonstar]

Thanks for the report, and reminder, I just merged in #8, and published v0.2.4. ❤️

[steffahn]

Ah, there was a PR already.. I didn't look at PRs when reporting this, so I wasn't aware ;-)

However, the bounds are overly restrictive now, which might be problematic, since this is a semver-minor version bump, so better introduce only as little breakage as necessary for fixing soundness. The Send implementation should be bound on T: Send and the Sync one on T: Sync.

This can be achieved by adding a set of manual unsafe impls.

unsafe impl<'a, T: Sync> Sync for Locked<'a, T> {}
unsafe impl<'a, T: Send> Send for Locked<'a, T> {}