Fix some sync sender/receiver undefined behavior.

[adrian-budau]

• Changed SyncSignal member in Signal to be a *const ptr so as to not have double mutable reference live at the same time (from both receiver and sender).
• Cloned the thread before unparking it in send/recv/terminate otherwise the owning thread might be scheduled during the unpark and destroy it. It's pretty hard to reproduce but the UB can be inferred from the Thread::unpark source code.

[fereidani]

Thanks, Adrian! looks good to me but just before merge I don't understand why you changed pub unsafe fn send(&self, d: T) to pub unsafe fn send(this: *const Self, d: T) what's the difference?

[fereidani]

Isn't it better to do this instead:
let thread=self.thread.clone(); self.state.force_unlock(); thread.unpark();

[adrian-budau]

Short story: Miri complained about it

Long story (I hope I got it right):

• Before: the function was passed as (**sig).send(d), this gets transformed to roughly:

let sig_ref = &**sig;
SyncSignal::send(sig_ref, d)

• This makes sig_ref semantically live as long as the function call. So the reference inside the function call (&self) will live as long as it's entire scope, it can not expire early. This means that sometime during the call to thread.unpark() the reference to &self would become invalid.

Manually inlining the call would actually fix it, because sig_ref would only have to live until it's last use (which would be sig_ref.thread.clone() or sig_ref.state.force_unlock() if I apply your previous comment fix (which makes sense, I'll update the PR).

[fereidani]

So I think it's best we can do right now, I'm gonna merge it if someone can improve the code to look better with less direct pointer access, please send PR.

[fereidani]

Thanks for your contribution.