From e9733f4c3523298de74b8fe25201f179b87f84b7 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 26 Jan 2021 15:28:55 -0800 Subject: [PATCH 1/7] io: fix memory leak on shutdown In some cases, a cycle is created between I/O driver wakers and the I/O driver resource slab. This patch clears stored wakers when an I/O resource is dropped, breaking the cycle. Fixes #3228 --- .github/workflows/ci.yml | 22 ++++++++++++++++++++++ tests-integration/Cargo.toml | 12 ++++++++++-- tests-integration/src/bin/test-mem.rs | 21 +++++++++++++++++++++ tokio/src/io/driver/registration.rs | 6 ++++++ tokio/src/io/driver/scheduled_io.rs | 6 ++++++ 5 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests-integration/src/bin/test-mem.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8640a9af19..5c40d8bf506 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,7 @@ jobs: - clippy - docs - loom + - valgrind steps: - run: exit 0 @@ -67,6 +68,27 @@ jobs: run: cargo hack test --each-feature working-directory: tests-build + valgrind: + name: valgrind + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Install Rust + run: rustup update stable + + - name: Install Valgrind + run: | + sudo apt-get update -y + sudo apt-get install -y valgrind + + # Compile tests + - name: cargo build + run: cargo build -p tests-integration --bin test-mem + + # Run with valgrind + - name: Run valgrind + run: valgrind --leak-check=full --show-leak-kinds=all ./target/debug/test-mem + test-unstable: name: test tokio full --unstable runs-on: ${{ matrix.os }} diff --git a/tests-integration/Cargo.toml b/tests-integration/Cargo.toml index 105b9c61816..0b98ddc54f9 100644 --- a/tests-integration/Cargo.toml +++ b/tests-integration/Cargo.toml @@ -5,7 +5,17 @@ authors = ["Tokio Contributors "] edition = "2018" publish = false +[[bin]] +name = "test-cat" + +[[bin]] +name = "test-mem" +required-features = ["rt-net"] + [features] +# For mem check +rt-net = ["tokio/rt", "tokio/rt-multi-thread", "tokio/net"] + full = [ "macros", "rt", @@ -23,6 +33,4 @@ rt-multi-thread = ["rt", "tokio/rt-multi-thread"] tokio = { path = "../tokio" } tokio-test = { path = "../tokio-test", optional = true } doc-comment = "0.3.1" - -[dev-dependencies] futures = { version = "0.3.0", features = ["async-await"] } diff --git a/tests-integration/src/bin/test-mem.rs b/tests-integration/src/bin/test-mem.rs new file mode 100644 index 00000000000..32c16cd79f8 --- /dev/null +++ b/tests-integration/src/bin/test-mem.rs @@ -0,0 +1,21 @@ +use futures::future::poll_fn; + +fn main() { + let rt = tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_io() + .build() + .unwrap(); + + rt.block_on(async { + let listener = tokio::net::TcpListener::bind("0.0.0.0:0").await.unwrap(); + tokio::spawn(async move { + loop { + poll_fn(|cx| listener.poll_accept(cx)).await.unwrap(); + } + }); + }); + + std::thread::sleep(std::time::Duration::from_millis(50)); + drop(rt); +} \ No newline at end of file diff --git a/tokio/src/io/driver/registration.rs b/tokio/src/io/driver/registration.rs index 93125814f84..8a588513469 100644 --- a/tokio/src/io/driver/registration.rs +++ b/tokio/src/io/driver/registration.rs @@ -205,6 +205,12 @@ impl Registration { } } +impl Drop for Registration { + fn drop(&mut self) { + self.shared.clear_wakers(); + } +} + fn gone() -> io::Error { io::Error::new(io::ErrorKind::Other, "IO driver has terminated") } diff --git a/tokio/src/io/driver/scheduled_io.rs b/tokio/src/io/driver/scheduled_io.rs index 75d562327c1..71864b3e65b 100644 --- a/tokio/src/io/driver/scheduled_io.rs +++ b/tokio/src/io/driver/scheduled_io.rs @@ -355,6 +355,12 @@ impl ScheduledIo { // result isn't important let _ = self.set_readiness(None, Tick::Clear(event.tick), |curr| curr - mask_no_closed); } + + pub(crate) fn clear_wakers(&self) { + let mut waiters = self.waiters.lock(); + waiters.reader.take(); + waiters.writer.take(); + } } impl Drop for ScheduledIo { From dd9059e8a12f4777b30beeada6ddc7e7a8dbc4d0 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 27 Jan 2021 21:17:20 -0800 Subject: [PATCH 2/7] prepare v1.0.3 release --- tokio/CHANGELOG.md | 5 +++++ tokio/Cargo.toml | 4 ++-- tokio/src/lib.rs | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tokio/CHANGELOG.md b/tokio/CHANGELOG.md index a36212d5449..abed4c12ad3 100644 --- a/tokio/CHANGELOG.md +++ b/tokio/CHANGELOG.md @@ -1,3 +1,8 @@ +# 1.0.3 (January 28, 2020) + +### Fixed +- io: memory leak during shutdown (#3477). + # 1.0.2 (January 14, 2020) ### Fixed diff --git a/tokio/Cargo.toml b/tokio/Cargo.toml index f950a286d17..7060326d593 100644 --- a/tokio/Cargo.toml +++ b/tokio/Cargo.toml @@ -8,12 +8,12 @@ name = "tokio" # - README.md # - Update CHANGELOG.md. # - Create "v1.0.x" git tag. -version = "1.0.2" +version = "1.0.3" edition = "2018" authors = ["Tokio Contributors "] license = "MIT" readme = "README.md" -documentation = "https://docs.rs/tokio/1.0.2/tokio/" +documentation = "https://docs.rs/tokio/1.0.3/tokio/" repository = "https://github.com/tokio-rs/tokio" homepage = "https://tokio.rs" description = """ diff --git a/tokio/src/lib.rs b/tokio/src/lib.rs index 7b098c7610a..dcda1f72823 100644 --- a/tokio/src/lib.rs +++ b/tokio/src/lib.rs @@ -1,4 +1,4 @@ -#![doc(html_root_url = "https://docs.rs/tokio/1.0.2")] +#![doc(html_root_url = "https://docs.rs/tokio/1.0.3")] #![allow( clippy::cognitive_complexity, clippy::large_enum_variant, From 4b0bb73cc062008714da20a1b8bb3427f9ba0e1c Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 28 Jan 2021 10:27:11 -0800 Subject: [PATCH 3/7] bump ci From 28efe502c66a746eb25df946a9260e41198d0db2 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 28 Jan 2021 10:45:36 -0800 Subject: [PATCH 4/7] tweak CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c40d8bf506..e4741546af4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,7 +83,7 @@ jobs: # Compile tests - name: cargo build - run: cargo build -p tests-integration --bin test-mem + run: cargo build -p tests-integration --features rt-net --bin test-mem # Run with valgrind - name: Run valgrind From 64889842009531ea0b985b8a21456adb6b3a7365 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 28 Jan 2021 11:26:06 -0800 Subject: [PATCH 5/7] fix ci --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4741546af4..414dc1bd453 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,7 +83,8 @@ jobs: # Compile tests - name: cargo build - run: cargo build -p tests-integration --features rt-net --bin test-mem + run: cargo build --features rt-net --bin test-mem + working-directory: tests-integration # Run with valgrind - name: Run valgrind From 32169ed7ea91c292e85df3e5a4f70746f7e9ab20 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 28 Jan 2021 11:29:59 -0800 Subject: [PATCH 6/7] fmt --- tests-integration/src/bin/test-mem.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests-integration/src/bin/test-mem.rs b/tests-integration/src/bin/test-mem.rs index 32c16cd79f8..98aa971ac60 100644 --- a/tests-integration/src/bin/test-mem.rs +++ b/tests-integration/src/bin/test-mem.rs @@ -8,9 +8,9 @@ fn main() { .unwrap(); rt.block_on(async { - let listener = tokio::net::TcpListener::bind("0.0.0.0:0").await.unwrap(); + let listener = tokio::net::TcpListener::bind("0.0.0.0:0").await.unwrap(); tokio::spawn(async move { - loop { + loop { poll_fn(|cx| listener.poll_accept(cx)).await.unwrap(); } }); @@ -18,4 +18,4 @@ fn main() { std::thread::sleep(std::time::Duration::from_millis(50)); drop(rt); -} \ No newline at end of file +} From 2631719f6d5b74f5856bb7157410e3907f28bc89 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 28 Jan 2021 15:38:44 -0800 Subject: [PATCH 7/7] add a comment describing the issue --- tokio/src/io/driver/registration.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tokio/src/io/driver/registration.rs b/tokio/src/io/driver/registration.rs index 8a588513469..1451224598c 100644 --- a/tokio/src/io/driver/registration.rs +++ b/tokio/src/io/driver/registration.rs @@ -207,6 +207,13 @@ impl Registration { impl Drop for Registration { fn drop(&mut self) { + // It is possible for a cycle to be created between wakers stored in + // `ScheduledIo` instances and `Arc`. To break this + // cycle, wakers are cleared. This is an imperfect solution as it is + // possible to store a `Registration` in a waker. In this case, the + // cycle would remain. + // + // See tokio-rs/tokio#3481 for more details. self.shared.clear_wakers(); } }