@hawkw Any chance of getting this merged?

@utkarshgupta137 whoops, sorry, this one slipped past me. Let me take a look!

@utkarshgupta137 thanks for the PR, happy to merge it once CI passes! are you interested in adding send_timeout methods as well?

Also, we should probably update the README to no longer state that send_timeout methods won't be implemented, if we add them :)

It looks like the loom tests are currently failing because loom's simulated version of std::thread doesn't have a mocked park_timeout function: https://github.com/hawkw/thingbuf/actions/runs/4812666947/jobs/8568203063?pr=75

Let's add #[cfg(not(all(test, loom))] to the recv_timeout methods to fix the compiler errors for loom tests?

@utkarshgupta137 thanks for the PR, happy to merge it once CI passes! are you interested in adding send_timeout methods as well?

Also, we should probably update the README to no longer state that send_timeout methods won't be implemented, if we add them :)

thingbuf/README.md

Lines 76 to 79 in a0e5740

	- **You need a blocking channel with `send_timeout`** or **a blocking channel
	with a `select` operation**. I'm probably not going to implement these things.
	The blocking channel isn't particularly important to me compared to the async
	channel, and I _probably_ won't add a bunch of additional APIs to it.

Sure, I'll try to add them, but let's release this first since I actually need these :)

Sweet! Looking forward to a new release.

Hmm, it looks like the doctests that were added in this PR are not deterministic --- they sometimes fail spuriously (presumably based on OS scheduler behavior): https://github.com/hawkw/thingbuf/actions/runs/4813223393/jobs/8569477594#step:4:194

Can we fix this? I'd prefer to have examples that don't involve multiple threads being scheduled in the right order --- perhaps we can base our examples on std::sync::mpsc's recv_timeout examples?

Hmm, it looks like the doctests that were added in this PR are not deterministic --- they sometimes fail spuriously (presumably based on OS scheduler behavior): https://github.com/hawkw/thingbuf/actions/runs/4813223393/jobs/8569477594#step:4:194

I think just increasing the durations would fix the issue?

Can we fix this? I'd prefer to have examples that don't involve multiple threads being scheduled in the right order --- perhaps we can base our examples on std::sync::mpsc's recv_timeout examples?

Do you mean creating 2 different examples?

Can we fix this? I'd prefer to have examples that don't involve multiple threads being scheduled in the right order --- perhaps we can base our examples on std::sync::mpsc's recv_timeout examples?

Do you mean creating 2 different examples?

Yes, the other thing I would note is that the standard library's examples never actually spawn an additional thread. Instead, it demonstrates the timeout behavior by trying to receive from a channel that will never have a message sent to it. I would kind of prefer to write our examples this way, because it's more deterministic and the doctests should never spuriously fail depending on scheduler behavior.

Can we fix this? I'd prefer to have examples that don't involve multiple threads being scheduled in the right order --- perhaps we can base our examples on std::sync::mpsc's recv_timeout examples?

Do you mean creating 2 different examples?

Yes, the other thing I would note is that the standard library's examples never actually spawn an additional thread. Instead, it demonstrates the timeout behavior by trying to receive from a channel that will never have a message sent to it. I would kind of prefer to write our examples this way, because it's more deterministic and the doctests should never spuriously fail depending on scheduler behavior.

But they're spawning additional threads & depending on the scheduler behavior...
They just have longer durations.

Success

thread::spawn(move || {
    send.send('a').unwrap();
});

assert_eq!(
    recv.recv_timeout(Duration::from_millis(400)),
    Ok('a')
);

Failure:

thread::spawn(move || {
    thread::sleep(Duration::from_millis(800));
    send.send('a').unwrap();
});

assert_eq!(
    recv.recv_timeout(Duration::from_millis(400)),
    Err(mpsc::RecvTimeoutError::Timeout)
);

But they're spawning additional threads & depending on the scheduler behavior...
They just have longer durations.

...yup, you're right, I can't read, apparently. I think increasing the sleep duration to at least twice the timeout is probably sufficient, then!