BiLock hangs when .lock().await from multiple tasks

[cairijun]

Though the documentation says a pair of BiLocks can only be used by two "owners", the current API permits a BiLock object being shared by multiple tasks (Sync + Send and .lock(&self)), while the implementation stores only the last Waker in .poll_lock.

https://github.com/rust-lang/futures-rs/blob/cc670fdf41703282c1c124fdaa4083637d460096/futures-util/src/lock/bilock.rs#L97-L103

#[tokio::test]
async fn test_bilock() {
    let (l1, l2) = BiLock::new(0);
    let l1 = l1.lock().await;  // lock l1 to ensure l2.lock() blocks
    let l2 = Arc::new(l2);
    // l2 is shared by two tasks via Arc
    let tasks = (0..2)
        .map(|i| {
            let l2 = Arc::clone(&l2);
            tokio::spawn(async move {
                {
                    println!("{} locking", i);
                    l2.lock().await;
                    println!("{} locked", i);
                }
                println!("{} unlocked", i);
            })
        })
        .collect::<Vec<_>>();
    tokio::task::yield_now().await;
    drop(l1);
    for task in tasks {
        task.await.unwrap();
    }
}

Actual output

0 locking
1 locking
1 locked
1 unlocked
<- hang forever

Expected behavior

I'm not sure if "a BiLock being shared by multiple tasks`" is a legitimate usage, but at least the API doesn't stop people from doing this and the docs doen't seem clear enough about this. I suppose it should at least panic in this scenario.

[novacrazy]

I feel like the BiLock's API should take mutable references to self even if it uses atomic operations internally. It only makes sense for each half to have one exclusive owner.

EDIT: Though that would mess with things given the returned guard is tied to that borrow. So just treat them as mutable even if it isn’t.