From 1d016a83f25c0484c58abb9242f282222592330e Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 10 Jun 2022 04:29:27 +0000 Subject: [PATCH] Lint against panicky calls in async functions (#3250) ## Description Add a new lint to CI that attempts to detect calls to functions like `block_on` from async execution contexts. This lint was written from scratch exactly for this purpose, on my fork of Clippy: https://github.com/michaelsproul/rust-clippy/tree/disallow-from-async ## Additional Info - I've successfully detected the previous two issues we had with `block_on` by running the linter on the commits prior to each of these PRs: https://github.com/sigp/lighthouse/pull/3165, https://github.com/sigp/lighthouse/pull/3199. - The lint runs on CI with `continue-on-error: true` so that if it fails spuriously it won't block CI. - I think it would be good to merge this PR before https://github.com/sigp/lighthouse/pull/3244 so that we can lint the extensive executor-related changes in that PR. - I aim to upstream the lint to Clippy, at which point building a custom version of Clippy from my fork will no longer be necessary. I imagine this will take several weeks or months though, because the code is currently a bit hacky and will need some renovations to pass review. --- .github/custom/clippy.toml | 21 +++++++++++++++++++++ .github/workflows/test-suite.yml | 17 +++++++++++++++++ .gitignore | 1 + Makefile | 9 +++++++++ 4 files changed, 48 insertions(+) create mode 100644 .github/custom/clippy.toml diff --git a/.github/custom/clippy.toml b/.github/custom/clippy.toml new file mode 100644 index 000000000..df0950230 --- /dev/null +++ b/.github/custom/clippy.toml @@ -0,0 +1,21 @@ +disallowed-from-async-methods = [ + "tokio::runtime::Handle::block_on", + "tokio::runtime::Runtime::block_on", + "tokio::task::LocalSet::block_on", + "tokio::sync::Mutex::blocking_lock", + "tokio::sync::RwLock::blocking_read", + "tokio::sync::mpsc::Receiver::blocking_recv", + "tokio::sync::mpsc::UnboundedReceiver::blocking_recv", + "tokio::sync::oneshot::Receiver::blocking_recv", + "tokio::sync::mpsc::Sender::blocking_send", + "tokio::sync::RwLock::blocking_write", +] +async-wrapper-methods = [ + "tokio::runtime::Handle::spawn_blocking", + "task_executor::TaskExecutor::spawn_blocking", + "task_executor::TaskExecutor::spawn_blocking_handle", + "warp_utils::task::blocking_task", + "warp_utils::task::blocking_json_task", + "validator_client::http_api::blocking_signed_json_task", + "execution_layer::test_utils::MockServer::new", +] diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index da0bcb385..a58491d04 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -252,6 +252,23 @@ jobs: run: make lint - name: Certify Cargo.lock freshness run: git diff --exit-code Cargo.lock + disallowed-from-async-lint: + name: disallowed-from-async-lint + runs-on: ubuntu-latest + needs: cargo-fmt + continue-on-error: true + steps: + - uses: actions/checkout@v1 + - name: Install SigP Clippy fork + run: | + cd .. + git clone https://github.com/michaelsproul/rust-clippy.git + cd rust-clippy + git checkout 31a49666ccfcd7963b63345d6ce757c373f22c2a + cargo build --release --bin cargo-clippy --bin clippy-driver + cargo build --release --bin cargo-clippy --bin clippy-driver -Zunstable-options --out-dir $(rustc --print=sysroot)/bin + - name: Run Clippy with the disallowed-from-async lint + run: make nightly-lint check-msrv: name: check-msrv runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 9376efc76..9830ef39b 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ perf.data* *.tar.gz /bin genesis.ssz +/clippy.toml diff --git a/Makefile b/Makefile index 01fd45a4d..a97637bfd 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ AARCH64_TAG = "aarch64-unknown-linux-gnu" BUILD_PATH_AARCH64 = "target/$(AARCH64_TAG)/release" PINNED_NIGHTLY ?= nightly +CLIPPY_PINNED_NIGHTLY=nightly-2022-05-19 # List of all hard forks. This list is used to set env variables for several tests so that # they run for different forks. @@ -145,6 +146,14 @@ lint: -A clippy::upper-case-acronyms \ -A clippy::vec-init-then-push +# FIXME: fails if --release is added due to broken HTTP API tests +nightly-lint: + cp .github/custom/clippy.toml . + cargo +$(CLIPPY_PINNED_NIGHTLY) clippy --workspace --tests -- \ + -A clippy::all \ + -D clippy::disallowed_from_async + rm clippy.toml + # Runs the makefile in the `ef_tests` repo. # # May download and extract an archive of test vectors from the ethereum