extra::sync has racy functions

[mstewartgallus]

Consider the following function in extra::sync. There is a race condition where after the peek another thread can recv from the head port. This race condition looks like it would cause an accident an extremely small number of times because the peek, and the recv are so close together but when the action does occur it will cause the really bad, and unexpected (to the user of Waitqueue) effect of the signal method to block on head!

fn signal_waitqueue(q: &Waitqueue) -> bool {
    // The peek is mandatory to make sure recv doesn't block.
    if q.head.peek() {
        // Pop and send a wakeup signal. If the waiter was killed, its port
        // will have closed. Keep trying until we get a live task.
        if comm::try_send_one(q.head.recv(), ()) {
            true
        } else {
            signal_waitqueue(q)
        }
    } else {
        false
    }
}

[bblum]

All uses of signal_waitqueue() are protected by a mutex surrounding the waitqueue. You'll note that the only place Waitqueues appear in the data structure hierarchy is in SemInner, which always appears inside of an Exclusive.

The function could probably do with a comment explaining that, though.

[mstewartgallus]

Thanks. I feel silly. I'll fix it up when I submit my patches for giving a RAII interface to extra::arc.