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.

[FlorianUekermann]

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.

What about cases where the future doesn't actually block or is guaranteed to finish on its own?

I have called block_on in cases where I use an async api with something like Cursor<Vec<u8>> as AsyncWrite, which shouldn't block. Now I'm learning that this may panic if a user happens to call my code within async code. That's pretty bad.

This makes block_on unsuitable for any library code and the documentation contains no mention of it.

I guess this is the correct solution in some cases:

        let mut ctx = Context::from_waker(noop_waker_ref());
        match my_future.poll_unpin(&mut ctx) {
            Poll::Ready(r) => r,
            Poll::Pending => unreachable!(),
        }

But that may be a bit too difficult to come up with for someone who has nothing to do with async and just wants to use an async API in their mostly non-async world.

Not sure what the correct solution is cases where transient blocking may happen. Do I have to implement my own spinning executor now? I thought that is exactly what block_on solves.

[chessai]

This seems to affect quickcheck_async when using the filesystem or Cursor mocks: https://docs.rs/quickcheck_async/latest/src/quickcheck_async/lib.rs.html#73-119