Porting select3.go tests.

[sofiabelen]

First time contributor, asking for some guidance

Hello everyone, nice to meet you!

I understand that, given the current situation, maintainers are especially busy these days. That's why I wanted to take a moment to introduce myself before kindly asking for some guidance. I've been learning about EBR and crossbeam, I made a couple of blog posts with my findings. I still have lots to learn before making any meaningful contribution to the project, but I wanted to familiarize myself with the codebase by helping out with porting the remaining go tests. If anyone has a spare moment to help, I'd greatly appreciate it!

I've created this draft-PR with what I've managed so far working with select3, but I have some questions.

As I understand, the goal of porting the go tests is ultimately to verify that, with the help of some wrapping code to make crossbeam channels behave like go's, all the invariants still hold. That said, we are not always testing 1:1 parallels in behavior due to differences in the languages. Why do I write this? Because I'm trying to wrap my head around the fact that there's some "glue" code that makes crossbeam channels behave like go's, and my initial question was: up to which point are we testing this "glue" part vs actual crossbeam behavior. So, this is the assumption with which I've been working with in my mental model, please correct me if I'm wrong.

Question 1: (nil vs closed channel) poisoned mutex

My mental model for closed vs nil channels is:
- closed channels: channel whose senders have all been closed.
- nil channels: in go, it's a pointer to a channel that wasn't initialized (we simulate this behavior by returning Some(nil_s)/Some(nil_r) when sender/receiver are None)
The question is, when we're testing that 'sending to a closed channel panics', if we do it like this:

let _ = closed_ch2.tx().send(7);

we poison the mutex because we're essentially panicking while holding the lock:

fn send(&self, val: T) {
    self.inner
        .lock()
        .unwrap()
        .s
        .as_ref()
        .expect("sending into closed channel")  // panics
        .send(val)
        .unwrap();
}

The problem is, when trying to reuse the channel for the rest of the test, we can't because the mutex is poisoned.

My workaround was to panic outside of the lock:

fn send(&self, msg: T) {
    let s = {
        let inner = self.inner.lock().unwrap();
        inner.s.clone()
    };
    // panic without holding the mutex to avoid poisoning it (for the sake of the tests)
    let _ = s.expect("sending into closed channel").send(msg);
}

I mean, we could also just treat the 'poisoned' channel as unusable. However, from my understanding, in the closed channel doesn't get corrupted if used. My intent was staying faithful to the spirit of the go tests.

Question 2: select! can't be empty

'empty selects always block' -> the select! macro would just not compile if it's empty, so I assume it's safe to leave this test out but clarify with a comment. I mean, the fact that it just doesn't compile is an even stronger mechanism than 'always blocks.' Is this the right approach?

Question 3: nil vs closed channel

Right now, this one blocks because of a similar problem with the nil vs closed channel behavior. closed_ch_clone.tx() doesn't make a distinction between the case where the sender is None because the channel is closed vs when it's a nil channel.

let closed_ch_clone = closed_ch.clone();
test_panic(ALWAYS, move || {
    select! {
        send(closed_ch_clone.tx(), 7) -> _ => {}
    }
});

What I came up with is to create a separate tx_checked:

fn tx_checked(&self) -> Sender<T> {
    self.inner
        .lock()
        .unwrap_or_else(|p| p.into_inner())
        .s
        .clone()
        .expect("sending into closed channel")
}

But... none of this looks right. The underlying problem I see is that Chan<T> has no internal way to differentiating between a nil vs a closed channel. Maybe the best way is to use an enum instead of Option<Sender<T>>/Option<Receiver<T>>:

enum ChanSide<T> {
    Real(T), // T = Sender<T> or Receiver<T>
    Nil,
    Closed,
}

then,

fn tx(&self) -> Sender<T> {
    match &self.lock().s {
        ChanSide::Real(s) => s.clone(),
        ChanSide::Nil => self.lock().nil_s.clone(),
        ChanSide::Closed => panic!("sending into closed channel"),
    }
}

The problem with this is that it's a more invasive change and would affect other tests.