search

tokio-rs / loom

Concurrency permutation testing tool for Rust.
MIT License
2.14k stars 111 forks source link

Add a loom aware block_on for running futures to completion #148

Closed faern closed 4 years ago

faern commented 4 years ago

This solves https://github.com/tokio-rs/tokio/issues/2463

Adding a way to run a Future to completion inside a loom model. Allows using loom to test the correctness of Future::poll implementations.

Do you want to have this in loom? If not, I might publish it as loom-executor or something along those lines. But the further upstream it can be the better IMO.

Feedback on the implementation greatly appreciated!

faern commented 4 years ago

Since I first wrote this as a standalone crate, I had to rely only on the public API of loom. But I bet it can be implemented more efficiently/better with some internals if it were to be inside loom like this PR suggests. Probably something like park/unpark. But I'm going to wait for some feedback before putting any effort into that.

LucioFranco commented 4 years ago

I think it would make sense to include a loom executor in here, maybe even behind a futures feature flag.

faern commented 4 years ago

Hmm.... Wait a minute. There already is a future module containing a block_on 😮 Doesn't this already do exactly what this PR implements?

I have completely missed it, because it's not in the docs.rs documentation. We should probably make it build the documentation with all features enabled. Otherwise the extra features become very hard to discover.

LucioFranco commented 4 years ago

@faern yeah, that is probably also a good idea too :)

hawkw commented 4 years ago

Should probably also add the doc_cfg attributes to show the required feature flags in the docs. :)

faern commented 4 years ago

I can do that. Do you want me to pull in the doc_cfg dependency to make it work on stable? Or go with the solution tokio has where it passes --cfg docsrc only on docs.rs and relies on nightly for just that?

Personally I would not be a huge fan of pulling in quote and proc_macro2 into the dependency tree just for such a simple thing. But since you mentioned that crate name I thought you might prefer it. On the other hand tokio probably have the other solution for a reason 🤷

hawkw commented 4 years ago

I can do that. Do you want me to pull in the doc_cfg dependency to make it work on stable? Or go with the solution tokio has where it passes --cfg docsrc only on docs.rs and relies on nightly for just that?

Personally I would not be a huge fan of pulling in quote and proc_macro2 into the dependency tree just for such a simple thing. But since you mentioned that crate name I thought you might prefer it. On the other hand tokio probably have the other solution for a reason shrug

I think it's better to do what Tokio does and just enable the feature on nightly only --- I wasn't actually aware of the doc_cfg crate until you mentioned it, I was just referring to the RustDoc feature.

faern commented 4 years ago

Replaced by #151... Kind of