Skip to content

Commit

Permalink
rt: rename some confusing internal variables/fns (#5151)
Browse files Browse the repository at this point in the history
This patch does some internal renames to remove some confusion.

* `allow_blocking` is renamed to `allow_block_in_place` to indicate that
  the variable only impacts the `block_in_place()` function.

* `context::try_enter` is renamed to `context::try_set_current` to
  disambiguate between the various "enter" functions. This function only
  sets the runtime handle used by Tokio's public APIs. Entering a runtime
  is a separate operation.  # Please enter the commit message for your
  changes.

* `scheduler::Handle::enter()` is removed to consolidate methods that
  set the current context.
  • Loading branch information
carllerche committed Oct 31, 2022
1 parent a9d5eb2 commit b1f40f4
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 37 deletions.
8 changes: 4 additions & 4 deletions tokio/src/runtime/context.rs
Expand Up @@ -31,7 +31,7 @@ cfg_rt! {
use crate::runtime::TryCurrentError;

#[derive(Debug)]
pub(crate) struct EnterGuard {
pub(crate) struct SetCurrentGuard {
old_handle: Option<scheduler::Handle>,
old_seed: RngSeed,
}
Expand All @@ -47,21 +47,21 @@ cfg_rt! {
/// Sets this [`Handle`] as the current active [`Handle`].
///
/// [`Handle`]: crate::runtime::scheduler::Handle
pub(crate) fn try_enter(handle: &scheduler::Handle) -> Option<EnterGuard> {
pub(crate) fn try_set_current(handle: &scheduler::Handle) -> Option<SetCurrentGuard> {
let rng_seed = handle.seed_generator().next_seed();

CONTEXT.try_with(|ctx| {
let old_handle = ctx.scheduler.borrow_mut().replace(handle.clone());
let old_seed = ctx.rng.replace_seed(rng_seed);

EnterGuard {
SetCurrentGuard {
old_handle,
old_seed,
}
}).ok()
}

impl Drop for EnterGuard {
impl Drop for SetCurrentGuard {
fn drop(&mut self) {
CONTEXT.with(|ctx| {
*ctx.scheduler.borrow_mut() = self.old_handle.take();
Expand Down
26 changes: 13 additions & 13 deletions tokio/src/runtime/enter.rs
Expand Up @@ -6,7 +6,7 @@ use std::marker::PhantomData;
pub(crate) enum EnterContext {
#[cfg_attr(not(feature = "rt"), allow(dead_code))]
Entered {
allow_blocking: bool,
allow_block_in_place: bool,
},
NotEntered,
}
Expand All @@ -30,8 +30,8 @@ cfg_rt! {
/// Marks the current thread as being within the dynamic extent of an
/// executor.
#[track_caller]
pub(crate) fn enter(allow_blocking: bool) -> Enter {
if let Some(enter) = try_enter(allow_blocking) {
pub(crate) fn enter(allow_block_in_place: bool) -> Enter {
if let Some(enter) = try_enter(allow_block_in_place) {
return enter;
}

Expand All @@ -45,12 +45,12 @@ cfg_rt! {

/// Tries to enter a runtime context, returns `None` if already in a runtime
/// context.
pub(crate) fn try_enter(allow_blocking: bool) -> Option<Enter> {
pub(crate) fn try_enter(allow_block_in_place: bool) -> Option<Enter> {
ENTERED.with(|c| {
if c.get().is_entered() {
None
} else {
c.set(EnterContext::Entered { allow_blocking });
c.set(EnterContext::Entered { allow_block_in_place });
Some(Enter { _p: PhantomData })
}
})
Expand Down Expand Up @@ -92,35 +92,35 @@ cfg_rt_multi_thread! {

cfg_rt! {
/// Disallows blocking in the current runtime context until the guard is dropped.
pub(crate) fn disallow_blocking() -> DisallowBlockingGuard {
pub(crate) fn disallow_block_in_place() -> DisallowBlockInPlaceGuard {
let reset = ENTERED.with(|c| {
if let EnterContext::Entered {
allow_blocking: true,
allow_block_in_place: true,
} = c.get()
{
c.set(EnterContext::Entered {
allow_blocking: false,
allow_block_in_place: false,
});
true
} else {
false
}
});
DisallowBlockingGuard(reset)
DisallowBlockInPlaceGuard(reset)
}

pub(crate) struct DisallowBlockingGuard(bool);
impl Drop for DisallowBlockingGuard {
pub(crate) struct DisallowBlockInPlaceGuard(bool);
impl Drop for DisallowBlockInPlaceGuard {
fn drop(&mut self) {
if self.0 {
// XXX: Do we want some kind of assertion here, or is "best effort" okay?
ENTERED.with(|c| {
if let EnterContext::Entered {
allow_blocking: false,
allow_block_in_place: false,
} = c.get()
{
c.set(EnterContext::Entered {
allow_blocking: true,
allow_block_in_place: true,
});
}
})
Expand Down
7 changes: 5 additions & 2 deletions tokio/src/runtime/handle.rs
Expand Up @@ -29,7 +29,7 @@ use std::{error, fmt};
#[derive(Debug)]
#[must_use = "Creating and dropping a guard does nothing"]
pub struct EnterGuard<'a> {
_guard: context::EnterGuard,
_guard: context::SetCurrentGuard,
_handle_lifetime: PhantomData<&'a Handle>,
}

Expand All @@ -44,7 +44,10 @@ impl Handle {
/// [`tokio::spawn`]: fn@crate::spawn
pub fn enter(&self) -> EnterGuard<'_> {
EnterGuard {
_guard: self.inner.enter(),
_guard: match context::try_set_current(&self.inner) {
Some(guard) => guard,
None => panic!("{}", crate::util::error::THREAD_LOCAL_DESTROYED_ERROR),
},
_handle_lifetime: PhantomData,
}
}
Expand Down
2 changes: 1 addition & 1 deletion tokio/src/runtime/mod.rs
Expand Up @@ -636,7 +636,7 @@ cfg_rt! {
Scheduler::CurrentThread(current_thread) => {
// This ensures that tasks spawned on the current-thread
// runtime are dropped inside the runtime's context.
match context::try_enter(&self.handle.inner) {
match context::try_set_current(&self.handle.inner) {
Some(guard) => current_thread.set_context_guard(guard),
None => {
// The context thread-local has already been destroyed.
Expand Down
6 changes: 3 additions & 3 deletions tokio/src/runtime/scheduler/current_thread.rs
@@ -1,7 +1,7 @@
use crate::future::poll_fn;
use crate::loom::sync::atomic::AtomicBool;
use crate::loom::sync::{Arc, Mutex};
use crate::runtime::context::EnterGuard;
use crate::runtime::context::SetCurrentGuard;
use crate::runtime::driver::{self, Driver};
use crate::runtime::task::{self, JoinHandle, OwnedTasks, Schedule, Task};
use crate::runtime::{blocking, Config};
Expand Down Expand Up @@ -34,7 +34,7 @@ pub(crate) struct CurrentThread {
/// scheduler, it is changed to `Some` with the context being the runtime's
/// own context. This ensures that any tasks dropped in the `CurrentThread`'s
/// destructor run in that runtime's context.
context_guard: Option<EnterGuard>,
context_guard: Option<SetCurrentGuard>,
}

/// Handle to the current thread scheduler
Expand Down Expand Up @@ -201,7 +201,7 @@ impl CurrentThread {
})
}

pub(crate) fn set_context_guard(&mut self, guard: EnterGuard) {
pub(crate) fn set_context_guard(&mut self, guard: SetCurrentGuard) {
self.context_guard = Some(guard);
}
}
Expand Down
12 changes: 1 addition & 11 deletions tokio/src/runtime/scheduler/mod.rs
Expand Up @@ -45,7 +45,7 @@ cfg_rt! {
use crate::future::Future;
use crate::loom::sync::Arc;
use crate::runtime::{blocking, task::Id};
use crate::runtime::context::{self, EnterGuard};
use crate::runtime::context;
use crate::task::JoinHandle;
use crate::util::RngSeedGenerator;

Expand All @@ -58,16 +58,6 @@ cfg_rt! {
}
}

/// Sets this [`Handle`] as the current active [`Handle`].
///
/// [`Handle`]: Handle
pub(crate) fn enter(&self) -> EnterGuard {
match context::try_enter(self) {
Some(guard) => guard,
None => panic!("{}", crate::util::error::THREAD_LOCAL_DESTROYED_ERROR),
}
}

pub(crate) fn blocking_spawner(&self) -> &blocking::Spawner {
match self {
Handle::CurrentThread(h) => &h.blocking_spawner,
Expand Down
9 changes: 7 additions & 2 deletions tokio/src/runtime/scheduler/multi_thread/worker.rs
Expand Up @@ -283,11 +283,16 @@ where
// set up blocking.
had_entered = true;
}
(EnterContext::Entered { allow_blocking }, false) => {
(
EnterContext::Entered {
allow_block_in_place,
},
false,
) => {
// We are on an executor, but _not_ on the thread pool. That is
// _only_ okay if we are in a thread pool runtime's block_on
// method:
if allow_blocking {
if allow_block_in_place {
had_entered = true;
return Ok(());
} else {
Expand Down
2 changes: 1 addition & 1 deletion tokio/src/task/local.rs
Expand Up @@ -890,7 +890,7 @@ impl<T: Future> Future for RunUntil<'_, T> {
.waker
.register_by_ref(cx.waker());

let _no_blocking = crate::runtime::enter::disallow_blocking();
let _no_blocking = crate::runtime::enter::disallow_block_in_place();
let f = me.future;

if let Poll::Ready(output) = crate::coop::budget(|| f.poll(cx)) {
Expand Down

0 comments on commit b1f40f4

Please sign in to comment.