Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rt(alt): fix a number of concurrency bugs #5907

Merged
merged 11 commits into from
Aug 4, 2023
Merged

rt(alt): fix a number of concurrency bugs #5907

merged 11 commits into from
Aug 4, 2023

Conversation

carllerche
Copy link
Member

Expands loom coverage and fixes a number of bugs.

Closes #5888
Depends on: tokio-rs/loom#323

Expands loom coverage and fixes a number of bugs.
@carllerche carllerche added A-tokio Area: The main tokio crate C-request Category: A non-feature request, e.g.: metadata updates, optimization suggestion, etc. labels Aug 3, 2023
@github-actions github-actions bot added the R-loom-multi-thread-alt Run loom multi-thread alt tests on this PR label Aug 3, 2023
@@ -93,6 +93,8 @@ pub struct Builder {
/// How many ticks before yielding to the driver for timer and I/O events?
pub(super) event_interval: u32,

pub(super) local_queue_capacity: usize,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR makes the local queue capacity configurable but does not expose the option. At some point, it might, but right now, it is for tests.

@@ -511,7 +519,6 @@ impl Builder {
/// .build();
/// # }
/// ```
#[cfg(not(loom))]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will revert his. I don't think it is needed anymore.

@carllerche carllerche marked this pull request as draft August 3, 2023 20:33
@Darksonn Darksonn added the M-runtime Module: tokio/runtime label Aug 3, 2023
@carllerche
Copy link
Member Author

carllerche commented Aug 3, 2023

Edit: this was left over debugging code gone wrong.

Looks like there is another spot it is failing

thread 'tokio-runtime-worker' panicked at 'steal=596; real=600; tail=600', tokio/src/runtime/scheduler/multi_thread_alt/queue.rs:133:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/panicking.rs:67:14
   2: tokio::runtime::scheduler::multi_thread_alt::queue::Local<T>::push_back
             at ./src/runtime/scheduler/multi_thread_alt/queue.rs:133:13
   3: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_remote_task_batch_synced
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:816:9
   4: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_remote_task_batch
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:785:26
   5: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_notified_task
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:753:9
   6: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_task
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:702:26
   7: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::run
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:579:35
   8: tokio::runtime::scheduler::multi_thread_alt::worker::run::{{closure}}::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:511:23
   9: tokio::runtime::context::scoped::Scoped<T>::set
             at ./src/runtime/context/scoped.rs:40:9
  10: tokio::runtime::context::set_scheduler::{{closure}}
             at ./src/runtime/context.rs:176:26
  11: std::thread::local::LocalKey<T>::try_with
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/thread/local.rs:270:16
  12: std::thread::local::LocalKey<T>::with
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/thread/local.rs:246:9
  13: tokio::runtime::context::set_scheduler
             at ./src/runtime/context.rs:176:9
  14: tokio::runtime::scheduler::multi_thread_alt::worker::run::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:507:9
  15: tokio::runtime::context::runtime::enter_runtime
             at ./src/runtime/context/runtime.rs:65:16
  16: tokio::runtime::scheduler::multi_thread_alt::worker::run
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:496:5
  17: tokio::runtime::scheduler::multi_thread_alt::worker::create::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:343:49
  18: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
             at ./src/runtime/blocking/task.rs:42:21
  19: tokio::runtime::task::core::Core<T,S>::poll::{{closure}}
             at ./src/runtime/task/core.rs:334:17
  20: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at ./src/loom/std/unsafe_cell.rs:16:9
  21: tokio::runtime::task::core::Core<T,S>::poll
             at ./src/runtime/task/core.rs:323:13
  22: tokio::runtime::task::harness::poll_future::{{closure}}
             at ./src/runtime/task/harness.rs:485:19
  23: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/panic/unwind_safe.rs:271:9
  24: std::panicking::try::do_call
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:500:40
  25: __rust_try
  26: std::panicking::try
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:464:19
  27: std::panic::catch_unwind
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panic.rs:142:14
  28: tokio::runtime::task::harness::poll_future
             at ./src/runtime/task/harness.rs:473:18
  29: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at ./src/runtime/task/harness.rs:208:27
  30: tokio::runtime::task::harness::Harness<T,S>::poll
             at ./src/runtime/task/harness.rs:153:15
  31: tokio::runtime::task::raw::poll
             at ./src/runtime/task/raw.rs:276:5
  32: tokio::runtime::task::raw::RawTask::poll
             at ./src/runtime/task/raw.rs:200:18
  33: tokio::runtime::task::UnownedTask<S>::run
             at ./src/runtime/task/mod.rs:437:9
  34: tokio::runtime::blocking::pool::Task::run
             at ./src/runtime/blocking/pool.rs:159:9
  35: tokio::runtime::blocking::pool::Inner::run
             at ./src/runtime/blocking/pool.rs:513:17
  36: tokio::runtime::blocking::pool::Spawner::spawn_thread::{{closure}}
             at ./src/runtime/blocking/pool.rs:471:13
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
worker thread panicking; aborting process

@carllerche
Copy link
Member Author

The broken CI depends on #5908

@carllerche carllerche marked this pull request as ready for review August 4, 2023 16:04
@carllerche carllerche marked this pull request as draft August 4, 2023 16:04
@@ -58,6 +61,10 @@ impl Driver {
))
}

pub(crate) fn is_enabled(&self) -> bool {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is added so the scheduler can avoid the extra thread if there are no resources to drive.

@carllerche carllerche marked this pull request as ready for review August 4, 2023 16:46
@carllerche carllerche merged commit 8832e93 into master Aug 4, 2023
75 checks passed
@carllerche carllerche deleted the fix-rt-alt-bug branch August 4, 2023 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-request Category: A non-feature request, e.g.: metadata updates, optimization suggestion, etc. M-runtime Module: tokio/runtime R-loom-multi-thread-alt Run loom multi-thread alt tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI spuriously failed due to panic in tokio::rt_threaded_alt many_multishot_futures
2 participants