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

Bump tokio MSRV to 1.63 #5887

Merged
merged 12 commits into from
Jul 27, 2023
Merged

Bump tokio MSRV to 1.63 #5887

merged 12 commits into from
Jul 27, 2023

Conversation

NobodyXu
Copy link
Contributor

@NobodyXu NobodyXu commented Jul 22, 2023

Also

  • Remove build.rs
  • Remove build-dependency autocfg.
  • Bump dependency socket2 to v0.5.3

Motivation

Bumping tokio MSRV to 1.63 will make all features required available and thus removing the need for build.rs.

It will also make {Mutex, Notify, OnceCell, RwLock, Semaphore}::const_new available by default and also make it possible for {Mutex, Notify, OnceCell, RwLock, Semaphore}::new to be usable in const context (#5885).

Solution

Bump tokio MSRV to 1.63

Also cleanup `build.rs` and remove dependency `autocfg`.

Signed-off-by: Jiahao XU <[email protected]>
@github-actions github-actions bot added R-loom-current-thread Run loom current-thread tests on this PR R-loom-multi-thread Run loom multi-thread tests on this PR R-loom-multi-thread-alt Run loom multi-thread alt tests on this PR R-loom-sync Run loom sync tests on this PR labels Jul 22, 2023
README.md Outdated Show resolved Hide resolved
tokio/Cargo.toml Show resolved Hide resolved
@Darksonn Darksonn added the A-tokio Area: The main tokio crate label Jul 22, 2023
Co-authored-by: Alice Ryhl <[email protected]>
tokio/build.rs Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@NobodyXu NobodyXu requested a review from Darksonn July 22, 2023 11:51
@github-actions github-actions bot added the R-loom-time-driver Run loom time driver tests on this PR label Jul 22, 2023
Replace `tokio_wasi` with `target_os = "wasi"` and replace latter with
`cfg_is_wasm_not_wasi!` or
`all(target_family = "wasm", not(target_os = "wasi"))`.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Copy link
Contributor Author

CI spuriously failed due to same error as #5885 (comment)

--- STDERR:              tokio::rt_threaded_alt many_multishot_futures ---
thread 'tokio-runtime-worker' panicked at 'explicit panic', tokio/src/runtime/scheduler/multi_thread_alt/queue.rs:149:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:117:5
   3: tokio::runtime::scheduler::multi_thread_alt::queue::Local<T>::push_back
             at ./src/runtime/scheduler/multi_thread_alt/queue.rs:149:13
   4: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_remote_task_batch_synced
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:799:9
   5: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::do_park
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:1154:29
   6: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::park
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:1125:30
   7: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::next_task
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:707:46
   8: tokio::runtime::scheduler::multi_thread_alt::worker::Worker::run
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:570:35
   9: tokio::runtime::scheduler::multi_thread_alt::worker::run::{{closure}}::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:502:23
  10: tokio::runtime::context::scoped::Scoped<T>::set
             at ./src/runtime/context/scoped.rs:40:9
  11: tokio::runtime::context::set_scheduler::{{closure}}
             at ./src/runtime/context.rs:176:26
  12: std::thread::local::LocalKey<T>::try_with
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/thread/local.rs:270:16
  13: std::thread::local::LocalKey<T>::with
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/thread/local.rs:246:9
  14: tokio::runtime::context::set_scheduler
             at ./src/runtime/context.rs:176:9
  15: tokio::runtime::scheduler::multi_thread_alt::worker::run::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:498:9
  16: tokio::runtime::context::runtime::enter_runtime
             at ./src/runtime/context/runtime.rs:65:16
  17: tokio::runtime::scheduler::multi_thread_alt::worker::run
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:487:5
  18: tokio::runtime::scheduler::multi_thread_alt::worker::create::{{closure}}
             at ./src/runtime/scheduler/multi_thread_alt/worker.rs:334:49
  19: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
             at ./src/runtime/blocking/task.rs:42:21
  20: <tracing::instrument::Instrumented<T> as core::future::future::Future>::poll
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.37/src/instrument.rs:272:9
  21: tokio::runtime::task::core::Core<T,S>::poll::{{closure}}
             at ./src/runtime/task/core.rs:334:17
  22: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at ./src/loom/std/unsafe_cell.rs:16:9
  23: tokio::runtime::task::core::Core<T,S>::poll
             at ./src/runtime/task/core.rs:323:13
  24: tokio::runtime::task::harness::poll_future::{{closure}}
             at ./src/runtime/task/harness.rs:485:19
  25: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panic/unwind_safe.rs:271:9
  26: std::panicking::try::do_call
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
  27: __rust_try
  28: std::panicking::try
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
  29: std::panic::catch_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
  30: tokio::runtime::task::harness::poll_future
             at ./src/runtime/task/harness.rs:473:18
  31: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at ./src/runtime/task/harness.rs:208:27
  32: tokio::runtime::task::harness::Harness<T,S>::poll
             at ./src/runtime/task/harness.rs:153:15
  33: tokio::runtime::task::raw::poll
             at ./src/runtime/task/raw.rs:276:5
  34: tokio::runtime::task::raw::RawTask::poll
             at ./src/runtime/task/raw.rs:200:18
  35: tokio::runtime::task::UnownedTask<S>::run
             at ./src/runtime/task/mod.rs:437:9
  36: tokio::runtime::blocking::pool::Task::run
             at ./src/runtime/blocking/pool.rs:159:9
  37: tokio::runtime::blocking::pool::Inner::run
             at ./src/runtime/blocking/pool.rs:513:17
  38: 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

   Canceling due to test failure: 1 tests still running
        PASS [   0.073s] tokio::rt_threaded_alt many_oneshot_futures
        PASS [  12.033s] tokio::rt_threaded many_multishot_futures
------------
     Summary [  31.677s] 709/1164 tests run: 708 passed, 1 failed, 21 skipped
     SIGABRT [   6.435s] tokio::rt_threaded_alt many_multishot_futures

@Darksonn
Copy link
Contributor

Please file a bug for the CI failure.

@NobodyXu
Copy link
Contributor Author

Please file a bug for the CI failure.

I've opened issue #5888 for this.

tokio/src/runtime/coop.rs Outdated Show resolved Hide resolved
tokio/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tokio/build.rs Show resolved Hide resolved
tokio/src/runtime/coop.rs Outdated Show resolved Hide resolved
tokio/Cargo.toml Outdated Show resolved Hide resolved
By replacing `any(target_arch = "wasm32", target_arch = "wasm64")`
with `target_family = "wasm"`.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu NobodyXu requested a review from taiki-e July 25, 2023 13:00
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@@ -107,8 +104,8 @@ mio = { version = "0.8.6", optional = true, default-features = false }
num_cpus = { version = "1.8.0", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this not be replaced by std::thread::available_parallelism?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, this is outside the scope of this PR.

This PR is for bumping MSRV for tokio only, you can open another PR/issue/discussion for this.

Copy link
Member

@taiki-e taiki-e Jul 27, 2023

Choose a reason for hiding this comment

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

available_parallelism was stabilized with an incomplete implementation, and it is expected to cause regressions (e.g., OOM) in Rust 1.63 and smaller that doesn't have cgroupv1 support (rust-lang/rust#97925).

So I think we need to stick to num_cpus until MSRV becomes 1.64+.

@NobodyXu
Copy link
Contributor Author

Is there anything need to do for this PR to merge?

Or are we just waiting for others' review?

@Darksonn Darksonn merged commit c445e46 into tokio-rs:master Jul 27, 2023
80 checks passed
@NobodyXu NobodyXu deleted the bump-msrv branch July 27, 2023 09:01
@NobodyXu
Copy link
Contributor Author

Thank you very much for the review!

@bcardarella
Copy link
Contributor

Does Tokio have a regular release cadence where we can plan on this being in a release soon? This feature is critical to a project I'm working on.

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2023

No, we do not have a regular cadence for releases, but we were planning on making one soon. If you wish to speed up the process, then you can help us by submitting a PR that prepares the release. See #5650 for an example.

@bcardarella
Copy link
Contributor

bcardarella commented Aug 6, 2023

@Darksonn what is the criteria for what goes into the changelog and what should the next version bump be?

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2023

The next version number should be 1.30.0. As for what is included, well:

  1. All changes to the public API, including behavior changes, must be listed.
  2. For documentation changes, we skip small changes, but otherwise we include them.
  3. Changes to unstable parts of Tokio are mentioned in a separate section for unstable changes.
  4. Changes to CI and similar are not included.

I will review your PR, so if anything is missing or should be omitted, I will let you know.

@bcardarella
Copy link
Contributor

@Darksonn is there a particular commit the next version should be PR'd against or just master/HEAD?

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2023

The master branch is fine.

@bcardarella
Copy link
Contributor

Looking over your list of changelog requirements it seems I would need a very intimate familiarity with the code base and the API to determine what is and isn't a changed behavior. If you're using SEMVER and bumping the minor I assume there are only additive API changes and nothing breaking.

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2023

Just categorize them according to how they look to you at a first glance. There's no need to be worried about getting it wrong—I will review the PR and tell you what things to change, if any. I know that you're not familiar with the codebase.

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2023

You may assume that there are no breaking changes.

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 R-loom-current-thread Run loom current-thread tests on this PR R-loom-multi-thread Run loom multi-thread tests on this PR R-loom-multi-thread-alt Run loom multi-thread alt tests on this PR R-loom-sync Run loom sync tests on this PR R-loom-time-driver Run loom time driver tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants