Splitting a tokio-test Mock instance can cause deadlock/lockup

[brandonson]

Using a tokio_test::io::Mock to read and write simultaneously via tokio::io::split causes lockup if the Mock is waiting, and there's no data to read.

This is a simple test, enough to trigger it for me:

#[tokio::test]
async fn check_io() {
    let socket = tokio_test::io::Builder::new()
        .wait(Duration::from_millis(10))
        .write([0].as_slice())
        .build();

    let (mut recv, mut send) = tokio::io::split(socket);
    tokio::spawn(async move { recv.read_u8().await.unwrap() });
    send.write_u8(0).await.unwrap();
}

I'm not super familiar with the Mock API, but I'd expect this to finish after 10ms, possibly with an error due to the extra read. Instead, it hangs forever.

As far as I'm aware, this should not be a problem unless the Mock is blocking in a poll call of some sort, which may be a deeper issue?

Version

├── tokio v1.45.0
│   └── tokio-macros v2.5.0 (proc-macro)
└── tokio-openssl v0.6.5
    └── tokio v1.45.0 (*)
└── tokio v1.45.0 (*)
├── tokio v1.45.0 (*)
└── tokio-util v0.7.15
    └── tokio v1.45.0 (*)
├── tokio v1.45.0 (*)
└── tokio-test v0.4.4
    ├── tokio v1.45.0 (*)
    └── tokio-stream v0.1.17
        └── tokio v1.45.0 (*)

Platform Linux [...] 5.15.0-139-generic #149~20.04.1-Ubuntu SMP [...] x86_64 x86_64 x86_64 GNU/Linux

[ADD-SP]

https://github.com/tokio-rs/tokio/blob/ab3ff69cf2258a8c696b2dca89a2cef4ff114c1c/tokio-test/src/io.rs#L360-L368

https://github.com/tokio-rs/tokio/blob/ab3ff69cf2258a8c696b2dca89a2cef4ff114c1c/tokio-test/src/io.rs#L408-L416

poll_read and poll_write poll the exact same Sleep, so we can consider this timeline.

1. send.write_u8(0).await
2. poll_write registers a timer.
3. poll_write polls the registered Sleep, and its waker is registered.
4. poll_write returns Poll::Pending and is suspended.
5. recv.read_u8().await
6. poll_read polls the exact same Sleep, and replaces the poll_write's waker with poll_read's waker.
7. Now, poll_write lost its waker.
8. Runtime fires Sleep and wakes up the poll_read, but nothing to read.
9. poll_read hangs forever.
10. poll_write also hangs forever because it lost its waker.

If we go back to the docs.

Sequence a wait.

The next operation in the mock’s script will be to wait without doing so for duration amount of time.

tokio_test::io::Builder::wait

The 'next operation' should either be poll_write or poll_read, otherwise, the API design might surprise the downstream developer.

I think this is a bug, poll_read and poll_write should not poll the same Sleep.

[redjonzaci]

i was suggested to add a warning to split, what do you think of the following: /// # Warning /// /// Using split with certain mock implementations (such as tokio_test::io::Mock) /// may cause deadlocks if the mock is configured to wait and both halves are used /// simultaneously. This occurs because both halves share the same underlying stream /// protected by a Mutex, and concurrent operations can lead to lock contention /// with waiting mock states. /// /// Consider using alternatives like tokio::io::duplex() for testing scenarios /// that require simultaneous read/write operations.

[ADD-SP]

i was suggested to add a warning to split, what do you think of the following: /// # Warning /// /// Using split with certain mock implementations (such as tokio_test::io::Mock) /// may cause deadlocks if the mock is configured to wait and both halves are used /// simultaneously. This occurs because both halves share the same underlying stream /// protected by a Mutex, and concurrent operations can lead to lock contention /// with waiting mock states. /// /// Consider using alternatives like tokio::io::duplex() for testing scenarios /// that require simultaneous read/write operations.

@redjonzaci These kind of warning is hard for people who is not familiar with tokio internal to understand. However, at least, we can add a short warning on the docs of tokio_test::io::Mock, just something like "Using spilit for Mock may cause deadlock, please don't do it".

[redjonzaci]

i just want us to agree on wording of the warning and i can add it in a PR, so we can then close this issue