Send bound needed on T (for Send/Sync impl of Producer/Consumer types)

[JOE1994]

Hello 🦀 ,
while scanning crates.io, we (Rust group @sslab-gatech) have noticed a soundness/memory safety issue in this crate which allows safe Rust code to trigger undefined behavior.

All queue types (spsc, spmc, mpsc, mpmc) are used to send an object from one thread to another.
Currently, Send/Sync is implemented for producer/consumer types of all queues without a Send bound on T (as below).

unsafe impl<T, B: Buffer<T>> Send for MPMCConsumer<T, B> {}
unsafe impl<T, B: Buffer<T>> Send for MPMCProducer<T, B> {}
unsafe impl<T, B: Buffer<T>> Sync for MPMCConsumer<T, B> {}
unsafe impl<T, B: Buffer<T>> Sync for MPMCProducer<T, B> {}

This allows users to send non-Send objects to other threads, and can potentially let users write safe Rust code that trigger undefined behavior.

Here is a proof of concept that invokes undefined behavior using this crate.
Due to the data race on the internal reference count of Rc, the program below either crashes at runtime
(On Ubuntu, Illegal Instruction (core dumped)) or panics at the end of the program (indicating a memory leak).

use magnetic::buffer::dynamic::DynamicBuffer;
use magnetic::{Producer, Consumer};
use magnetic::mpmc::mpmc_queue; 

use std::rc::Rc;

fn main() {
    let (p, c) = mpmc_queue(DynamicBuffer::new(32).unwrap());
    const N_ITER: usize = 2_000_000;

    // Send `Consumer` to child thread.
    let t1 = std::thread::spawn(move || {
        for _ in 0..N_ITER {
            // Decrements refcount of `Rc` w/o synchronization
            c.pop().unwrap();
        }
    });

    let original_rc = Rc::new(0_i32);
    for _ in 0..N_ITER {
        // Increments refcount of `Rc` w/o synchronization
        p.push(Rc::clone(&original_rc)).unwrap();
    }
    t1.join().unwrap();

    assert_eq!(Rc::strong_count(&original_rc), 1);
}

In order to prevent this undefined behavior, we need a Send bound on T for both Send/Sync impls of producer/consumer types.
Send bound is needed for impl Sync, due to the fact that it is possible to send objects to other threads by holding a reference to Producer/Consumer types.

I made a fix that lets the compiler revoke programs like the above, and I'll submit a PR right away.
Please let us know what you think about the issue or the fix.

Thank you for checking out this issue 🦀

[johnshaw]

Interesting work your doing @JOE1994 . Thanks for the PR.