From cc7be86317a1e686b749ef77fd6a79d2736faeb7 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 27 May 2020 16:20:12 +0200 Subject: [PATCH 1/4] fix: allow for recursive block-on calls --- Cargo.toml | 4 +++ src/task/builder.rs | 37 +++++++++++++++++++++++++++- tests/block_on.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d2daaf035..7acf75b02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,3 +103,7 @@ required-features = ["unstable"] [[example]] name = "surf-web" required-features = ["surf"] + +[patch.crates-io] +smol = { git = "https://github.com/dignifiedquire/smol-1", branch = "feat/recursive-block-on" } + diff --git a/src/task/builder.rs b/src/task/builder.rs index cbb3187f2..6bc9b181d 100644 --- a/src/task/builder.rs +++ b/src/task/builder.rs @@ -1,3 +1,4 @@ +use std::cell::Cell; use std::future::Future; use std::pin::Pin; use std::sync::Arc; @@ -8,6 +9,11 @@ use pin_project_lite::pin_project; use crate::io; use crate::task::{JoinHandle, Task, TaskLocalsWrapper}; +#[cfg(not(target_os = "unknown"))] +thread_local! { + static IS_CURRENT_BLOCKING: Cell = Cell::new(0); +} + /// Task builder that configures the settings of a new task. #[derive(Debug, Default)] pub struct Builder { @@ -151,7 +157,36 @@ impl Builder { }); // Run the future as a task. - unsafe { TaskLocalsWrapper::set_current(&wrapped.tag, || smol::run(wrapped)) } + IS_CURRENT_BLOCKING.with(|is_current_blocking| { + let count = is_current_blocking.get(); + let res = if count == 0 { + // increase the count + is_current_blocking.replace(1); + + // The first call should use run. + unsafe { + TaskLocalsWrapper::set_current(&wrapped.tag, || { + let res = smol::run(wrapped); + is_current_blocking.replace(is_current_blocking.get() - 1); + res + }) + } + } else { + // increase the count + is_current_blocking.replace(is_current_blocking.get() + 1); + + // Subsequent calls should be using `block_on`. + unsafe { + TaskLocalsWrapper::set_current(&wrapped.tag, || { + let res = smol::block_on(wrapped); + is_current_blocking.replace(is_current_blocking.get() - 1); + res + }) + } + }; + + res + }) } } diff --git a/tests/block_on.rs b/tests/block_on.rs index 28902b018..101fb2022 100644 --- a/tests/block_on.rs +++ b/tests/block_on.rs @@ -16,3 +16,62 @@ fn panic() { panic!("boom"); }); } + +#[cfg(feature = "unstable")] +#[test] +fn nested_block_on_local() { + let x = task::block_on(async { + let a = + task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); + let b = task::spawn_local(async { + task::block_on(async { async_std::future::ready(2).await }) + }) + .await; + let c = + task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + a + b + c + }); + + assert_eq!(x, 3 + 2 + 1); + + let y = task::block_on(async { + let a = + task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); + let b = task::spawn_local(async { + task::block_on(async { async_std::future::ready(2).await }) + }) + .await; + let c = + task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + a + b + c + }); + + assert_eq!(y, 3 + 2 + 1); +} + +#[test] +fn nested_block_on() { + let x = task::block_on(async { + let a = + task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); + let b = + task::block_on(async { task::block_on(async { async_std::future::ready(2).await }) }); + let c = + task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + a + b + c + }); + + assert_eq!(x, 3 + 2 + 1); + + let y = task::block_on(async { + let a = + task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); + let b = + task::block_on(async { task::block_on(async { async_std::future::ready(2).await }) }); + let c = + task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + a + b + c + }); + + assert_eq!(y, 3 + 2 + 1); +} From 09840dddec9860e546e92a58f518f9997917850c Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Thu, 4 Jun 2020 12:26:28 +0200 Subject: [PATCH 2/4] switch to released smol --- Cargo.toml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7acf75b02..284c8b813 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,7 +75,7 @@ futures-timer = { version = "3.0.2", optional = true } surf = { version = "1.0.3", optional = true } [target.'cfg(not(target_os = "unknown"))'.dependencies] -smol = { version = "0.1.10", optional = true } +smol = { version = "0.1.11", optional = true } [target.'cfg(target_arch = "wasm32")'.dependencies] futures-timer = { version = "3.0.2", optional = true, features = ["wasm-bindgen"] } @@ -104,6 +104,3 @@ required-features = ["unstable"] name = "surf-web" required-features = ["surf"] -[patch.crates-io] -smol = { git = "https://github.com/dignifiedquire/smol-1", branch = "feat/recursive-block-on" } - From b9eb213548140f05d98bbbe235bcfccc47dab255 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Thu, 4 Jun 2020 12:54:31 +0200 Subject: [PATCH 3/4] apply CR --- src/task/builder.rs | 57 ++++++++++++++++++--------------------------- tests/block_on.rs | 57 +++++++++++++++++---------------------------- 2 files changed, 45 insertions(+), 69 deletions(-) diff --git a/src/task/builder.rs b/src/task/builder.rs index 6bc9b181d..0024f8ab8 100644 --- a/src/task/builder.rs +++ b/src/task/builder.rs @@ -9,11 +9,6 @@ use pin_project_lite::pin_project; use crate::io; use crate::task::{JoinHandle, Task, TaskLocalsWrapper}; -#[cfg(not(target_os = "unknown"))] -thread_local! { - static IS_CURRENT_BLOCKING: Cell = Cell::new(0); -} - /// Task builder that configures the settings of a new task. #[derive(Debug, Default)] pub struct Builder { @@ -156,36 +151,30 @@ impl Builder { parent_task_id: TaskLocalsWrapper::get_current(|t| t.id().0).unwrap_or(0), }); + thread_local! { + /// Tracks the number of nested block_on calls. + static NUM_NESTED_BLOCKING: Cell = Cell::new(0); + } + // Run the future as a task. - IS_CURRENT_BLOCKING.with(|is_current_blocking| { - let count = is_current_blocking.get(); - let res = if count == 0 { - // increase the count - is_current_blocking.replace(1); - - // The first call should use run. - unsafe { - TaskLocalsWrapper::set_current(&wrapped.tag, || { - let res = smol::run(wrapped); - is_current_blocking.replace(is_current_blocking.get() - 1); - res - }) - } - } else { - // increase the count - is_current_blocking.replace(is_current_blocking.get() + 1); - - // Subsequent calls should be using `block_on`. - unsafe { - TaskLocalsWrapper::set_current(&wrapped.tag, || { - let res = smol::block_on(wrapped); - is_current_blocking.replace(is_current_blocking.get() - 1); - res - }) - } - }; - - res + NUM_NESTED_BLOCKING.with(|num_nested_blocking| { + let count = num_nested_blocking.get(); + let should_run = count == 0; + // increase the count + num_nested_blocking.replace(count + 1); + + unsafe { + TaskLocalsWrapper::set_current(&wrapped.tag, || { + let res = if should_run { + // The first call should use run. + smol::run(wrapped) + } else { + smol::block_on(wrapped) + }; + num_nested_blocking.replace(num_nested_blocking.get() - 1); + res + }) + } }) } } diff --git a/tests/block_on.rs b/tests/block_on.rs index 101fb2022..aeb1d213c 100644 --- a/tests/block_on.rs +++ b/tests/block_on.rs @@ -1,17 +1,20 @@ #![cfg(not(target_os = "unknown"))] -use async_std::task; +use async_std::{ + future::ready, + task::{block_on, spawn_local}, +}; #[test] fn smoke() { - let res = task::block_on(async { 1 + 2 }); + let res = block_on(async { 1 + 2 }); assert_eq!(res, 3); } #[test] #[should_panic = "boom"] fn panic() { - task::block_on(async { + block_on(async { // This panic should get propagated into the parent thread. panic!("boom"); }); @@ -20,29 +23,19 @@ fn panic() { #[cfg(feature = "unstable")] #[test] fn nested_block_on_local() { - let x = task::block_on(async { - let a = - task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); - let b = task::spawn_local(async { - task::block_on(async { async_std::future::ready(2).await }) - }) - .await; - let c = - task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + let x = block_on(async { + let a = block_on(async { block_on(async { ready(3).await }) }); + let b = spawn_local(async { block_on(async { ready(2).await }) }).await; + let c = block_on(async { block_on(async { ready(1).await }) }); a + b + c }); assert_eq!(x, 3 + 2 + 1); - let y = task::block_on(async { - let a = - task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); - let b = task::spawn_local(async { - task::block_on(async { async_std::future::ready(2).await }) - }) - .await; - let c = - task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + let y = block_on(async { + let a = block_on(async { block_on(async { ready(3).await }) }); + let b = spawn_local(async { block_on(async { ready(2).await }) }).await; + let c = block_on(async { block_on(async { ready(1).await }) }); a + b + c }); @@ -51,25 +44,19 @@ fn nested_block_on_local() { #[test] fn nested_block_on() { - let x = task::block_on(async { - let a = - task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); - let b = - task::block_on(async { task::block_on(async { async_std::future::ready(2).await }) }); - let c = - task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + let x = block_on(async { + let a = block_on(async { block_on(async { ready(3).await }) }); + let b = block_on(async { block_on(async { ready(2).await }) }); + let c = block_on(async { block_on(async { ready(1).await }) }); a + b + c }); assert_eq!(x, 3 + 2 + 1); - let y = task::block_on(async { - let a = - task::block_on(async { task::block_on(async { async_std::future::ready(3).await }) }); - let b = - task::block_on(async { task::block_on(async { async_std::future::ready(2).await }) }); - let c = - task::block_on(async { task::block_on(async { async_std::future::ready(1).await }) }); + let y = block_on(async { + let a = block_on(async { block_on(async { ready(3).await }) }); + let b = block_on(async { block_on(async { ready(2).await }) }); + let c = block_on(async { block_on(async { ready(1).await }) }); a + b + c }); From d3cf62d001eec8d0a6b401ee2d378c4532fbf711 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Thu, 4 Jun 2020 13:09:29 +0200 Subject: [PATCH 4/4] fixup imports --- tests/block_on.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/block_on.rs b/tests/block_on.rs index aeb1d213c..4c264804d 100644 --- a/tests/block_on.rs +++ b/tests/block_on.rs @@ -1,9 +1,6 @@ #![cfg(not(target_os = "unknown"))] -use async_std::{ - future::ready, - task::{block_on, spawn_local}, -}; +use async_std::{future::ready, task::block_on}; #[test] fn smoke() { @@ -23,6 +20,8 @@ fn panic() { #[cfg(feature = "unstable")] #[test] fn nested_block_on_local() { + use async_std::task::spawn_local; + let x = block_on(async { let a = block_on(async { block_on(async { ready(3).await }) }); let b = spawn_local(async { block_on(async { ready(2).await }) }).await;