Futures-rs: Recursive use of block_on panics

[That3Percent]

When futures::executor::block_on is called recursively it panics with:

thread 'main' panicked at 'cannot execute `LocalPool` executor from within another executor: EnterError', src/libcore/result.rs:1188:5

Here is a complete program to reproduce this result:

fn main() {
    use futures::executor::block_on;
    block_on(async { block_on( async {} ) })
}

The documentation for block_on does not mention any limitation that would cause a panic, and it is unclear (to me at least) that such a limitation would be necessary.

The actual code that causes this panic is here in local_pool.rs

[yoshuawuyts]

I believe our implementation of block on in async-std handles this case: impl here. Maybe it could make sense for futures-rs to adopt a similar fix?

[seanmonstar]

It purposefully panics because a recursive call means blocking any other futures that were running on the same thread.

[cramertj]

Yeah, @seanmonstar is right-- this is "working as intended". It's quite bugprone (or arguably always wrong) to block inside the implementation of a future. I'll leave this issue open for discussion, but I don't personally think there's a bug to be fixed here.

[That3Percent]

Yes, I'm coming around and think that the panic here is reasonable and desirable.

I was originally thinking that at the very least the docs should be amended to document the potential panic - but the more I think about it "Future.poll must never block" is documented and must be well-known and adhered to across the ecosystem. Violating that constraint could cause anything to happen (undefined behavior excepted) in library code - even panics.

[kevincox]

I have a use case where I know that the parent executor is idle and need to interact with some code that isn't async. It would be nice to be able to do this for the weird cases where it is needed. I know that it isn't the "perfect" solution in most cases but when you need it, well, you need it.

[tgiddings]

If block_on purposely panics, it should give a relevent panic message. The current one is phrased in terms of the internals of block_on, which is not informative and implies that the panic is not correct.

[taiki-e]

We can allow this once the lint like the one that this shiny future story mentions is implemented.