Re: [pbs-devel] [PATCH proxmox] async: runtime: fix `block_in_place` panicking in wrong runtime

[Max Carrara <m.carrara@proxmox.com>]

On 8/18/23 09:26, Wolfgang Bumiller wrote:
> Does a single threaded runtime support `.spawn_blocking()`? Maybe it
> would make sense to use that in this case?
> 

It does, yes!

However, we can't actually use `spawn_blocking()` inside our
`block_in_place()` wrapper; the former is async, the latter is sync.
So, I'll think of some other alternatives.

Maybe some additional helpers would work instead? Something like:

  fn try_block_in_place<F, R>(func: F) -> Result<R, F>
  where
      F: FnOnce() -> R
  {
      ...
  }

Basically returning the original closure if
`tokio::task::block_in_place()` cannot be called.

> Because this just seems a tiny bit dangerous.
> Then again, `block_in_place` is the wrong helper if the blocking
> operation also depends on any futures making progress, it should only be
> used for independent operations... but that doesn't mean it can't
> accidentally happen somehow...

I'm not sure there's any way to prevent this from happening
accidentally. Maybe we can document this to make it apparent when
`block_in_place()` is appropriate and when it isn't? I'd include that
in the v2 in that case.


FWIW, there's a way[0] to re-enter an async context while within
`block_in_place()` using `Handle::block_on()`[1]:

  use tokio::task;
  use tokio::runtime::Handle;

  task::block_in_place(move || {
      Handle::current().block_on(async move {
          // do something async
      });
  });

This isn't really ideal either imo (because why block when you depend
on other futures? There are better ways to do that) but I guess it's
useful to know.


[0]:
https://docs.rs/tokio/latest/tokio/task/fn.block_in_place.html#examples
[1]:
https://docs.rs/tokio/latest/tokio/runtime/struct.Handle.html#method.block_on


> Ideally we could get rid of all the block-in-place stuff without slowing
> things down too much, but the latter part is difficult :-)
> 
> On Thu, Aug 17, 2023 at 06:46:37PM +0200, Max Carrara wrote:
>> Because `tokio::task::block_in_place` panics if called in the
>> "current_thread" runtime, so does our wrapper function.
>>
>> This is fixed by checking whether we're actually in a multithreaded
>> tokio runtime.
>>
>> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
>> ---
>>  proxmox-async/src/runtime.rs | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/proxmox-async/src/runtime.rs b/proxmox-async/src/runtime.rs
>> index 0fe9fae..35cf7c3 100644
>> --- a/proxmox-async/src/runtime.rs
>> +++ b/proxmox-async/src/runtime.rs
>> @@ -15,7 +15,12 @@ thread_local! {
>>  }
>>
>>  fn is_in_tokio() -> bool {
>> -    tokio::runtime::Handle::try_current().is_ok()
>> +    tokio::runtime::Handle::try_current().is_ok_and(|rt_handle| {
>> +        matches!(
>> +            rt_handle.runtime_flavor(),
>> +            tokio::runtime::RuntimeFlavor::MultiThread
>> +        )
>> +    })
>>  }
>>
>>  fn is_blocking() -> bool {
>> --
>> 2.39.2