From a78380739e85049c0ef890deb6fcb0cd8de9bc8b Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 9 Aug 2020 13:32:37 +0100 Subject: [PATCH 1/4] require rustls feature for client example (#1625) --- .gitignore | 1 + Cargo.toml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 42d0755dd..11a3b5f37 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ guide/build/ *.pid *.sock *~ +.DS_Store # These are backup files generated by rustfmt **/*.rs.bk diff --git a/Cargo.toml b/Cargo.toml index 49de9fd96..afaeac09d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -124,6 +124,10 @@ actix-files = { path = "actix-files" } actix-multipart = { path = "actix-multipart" } awc = { path = "awc" } +[[example]] +name = "client" +required-features = ["rustls"] + [[bench]] name = "server" harness = false From 46627be36ff06aa42e907f1dc223b09b3f93387c Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 9 Aug 2020 13:54:35 +0100 Subject: [PATCH 2/4] add dep graph dot graphs (#1601) --- docs/graphs/.gitignore | 2 ++ docs/graphs/dependency-graphs.md | 11 +++++++++++ docs/graphs/net-only.dot | 25 +++++++++++++++++++++++++ docs/graphs/web-focus.dot | 30 ++++++++++++++++++++++++++++++ docs/graphs/web-only.dot | 19 +++++++++++++++++++ 5 files changed, 87 insertions(+) create mode 100644 docs/graphs/.gitignore create mode 100644 docs/graphs/dependency-graphs.md create mode 100644 docs/graphs/net-only.dot create mode 100644 docs/graphs/web-focus.dot create mode 100644 docs/graphs/web-only.dot diff --git a/docs/graphs/.gitignore b/docs/graphs/.gitignore new file mode 100644 index 000000000..284a286c9 --- /dev/null +++ b/docs/graphs/.gitignore @@ -0,0 +1,2 @@ +# do not track rendered graphs +*.png diff --git a/docs/graphs/dependency-graphs.md b/docs/graphs/dependency-graphs.md new file mode 100644 index 000000000..6a4392d6b --- /dev/null +++ b/docs/graphs/dependency-graphs.md @@ -0,0 +1,11 @@ +# Actix Ecosystem Dependency Graphs + +See rendered versions of these dot graphs [on the wiki](https://github.com/actix/actix-web/wiki/Dependency-Graph). + +## Rendering + +Dot graphs were rendered using the `dot` command from [GraphViz](https://www.graphviz.org/doc/info/command.html): + +```sh +for f in $(ls docs/graphs/*.dot | xargs); do dot $f -Tpng -o${f:r}.png; done +``` diff --git a/docs/graphs/net-only.dot b/docs/graphs/net-only.dot new file mode 100644 index 000000000..d9f2317a1 --- /dev/null +++ b/docs/graphs/net-only.dot @@ -0,0 +1,25 @@ +digraph { + subgraph cluster_net { + label="actix/actix-net"; + "actix-codec" + "actix-connect" + "actix-macros" + "actix-rt" + "actix-server" + "actix-service" + "actix-testing" + "actix-threadpool" + "actix-tls" + "actix-tracing" + "actix-utils" + "actix-router" + } + + "actix-utils" -> { "actix-service" "actix-rt" "actix-codec" } + "actix-tracing" -> { "actix-service" } + "actix-tls" -> { "actix-service" "actix-codec" "actix-utils" "actix-rt" } + "actix-testing" -> { "actix-rt" "actix-macros" "actix-server" "actix-service" } + "actix-server" -> { "actix-service" "actix-rt" "actix-codec" "actix-utils" } + "actix-rt" -> { "actix-macros" "actix-threadpool" } + "actix-connect" -> { "actix-service" "actix-codec" "actix-utils" "actix-rt" } +} diff --git a/docs/graphs/web-focus.dot b/docs/graphs/web-focus.dot new file mode 100644 index 000000000..b0ce18d02 --- /dev/null +++ b/docs/graphs/web-focus.dot @@ -0,0 +1,30 @@ +digraph { + subgraph cluster_web { + label="actix/actix-web" + "awc" + "actix-web" + "actix-files" + "actix-http" + "actix-multipart" + "actix-web-actors" + "actix-web-codegen" + } + + "actix-web" -> { "actix-codec" "actix-service" "actix-utils" "actix-router" "actix-rt" "actix-server" "actix-testing" "actix-macros" "actix-threadpool" "actix-tls" "actix-web-codegen" "actix-http" "awc" } + "awc" -> { "actix-codec" "actix-service" "actix-http" "actix-rt" } + "actix-web-actors" -> { "actix" "actix-web" "actix-http" "actix-codec" } + "actix-multipart" -> { "actix-web" "actix-service" "actix-utils" } + "actix-http" -> { "actix-service" "actix-codec" "actix-connect" "actix-utils" "actix-rt" "actix-threadpool" } + "actix-http" -> { "actix" "actix-tls" }[color=blue] // optional + "actix-files" -> { "actix-web" "actix-http" } + + // net + + "actix-utils" -> { "actix-service" "actix-rt" "actix-codec" } + "actix-tracing" -> { "actix-service" } + "actix-tls" -> { "actix-service" "actix-codec" "actix-utils" "actix-rt" } + "actix-testing" -> { "actix-rt" "actix-macros" "actix-server" "actix-service" } + "actix-server" -> { "actix-service" "actix-rt" "actix-codec" "actix-utils" } + "actix-rt" -> { "actix-macros" "actix-threadpool" } + "actix-connect" -> { "actix-service" "actix-codec" "actix-utils" "actix-rt" } +} diff --git a/docs/graphs/web-only.dot b/docs/graphs/web-only.dot new file mode 100644 index 000000000..6e41fdc27 --- /dev/null +++ b/docs/graphs/web-only.dot @@ -0,0 +1,19 @@ +digraph { + subgraph cluster_web { + label="actix/actix-web" + "awc" + "actix-web" + "actix-files" + "actix-http" + "actix-multipart" + "actix-web-actors" + "actix-web-codegen" + } + + "actix-web" -> { "actix-web-codegen" "actix-http" "awc" } + "awc" -> { "actix-http" } + "actix-web-actors" -> { "actix" "actix-web" "actix-http" } + "actix-multipart" -> { "actix-web" } + "actix-http" -> { "actix" }[color=blue] // optional + "actix-files" -> { "actix-web" "actix-http" } +} From 187646b2f9900ec56cbec6b4d1b168b1d0e78058 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 9 Aug 2020 15:51:38 +0100 Subject: [PATCH 3/4] match HttpRequest app_data behavior in ServiceRequest (#1618) --- CHANGES.md | 3 +++ src/service.rs | 32 ++++++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d80e87946..adc343050 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,8 @@ using `App::data`. [#1610] * `web::Path` now has a public representation: `web::Path(pub T)` that enables destructuring. [#1594] +* `ServiceRequest::app_data` allows retrieval of non-Data data without splitting into parts to + access `HttpRequest` which already allows this. [#1618] * MSRV is now 1.42.0. ### Fixed @@ -14,6 +16,7 @@ [#1594]: https://github.com/actix/actix-web/pull/1594 [#1609]: https://github.com/actix/actix-web/pull/1609 [#1610]: https://github.com/actix/actix-web/pull/1610 +[#1618]: https://github.com/actix/actix-web/pull/1610 ## 3.0.0-beta.1 - 2020-07-13 diff --git a/src/service.rs b/src/service.rs index cba852a9c..a861ba38c 100644 --- a/src/service.rs +++ b/src/service.rs @@ -12,7 +12,6 @@ use actix_router::{IntoPattern, Path, Resource, ResourceDef, Url}; use actix_service::{IntoServiceFactory, ServiceFactory}; use crate::config::{AppConfig, AppService}; -use crate::data::Data; use crate::dev::insert_slash; use crate::guard::Guard; use crate::info::ConnectionInfo; @@ -226,12 +225,11 @@ impl ServiceRequest { self.0.app_config() } - /// Get an application data stored with `App::data()` method during - /// application configuration. - pub fn app_data(&self) -> Option> { + /// Counterpart to [`HttpRequest::app_data`](../struct.HttpRequest.html#method.app_data). + pub fn app_data(&self) -> Option<&T> { for container in (self.0).0.app_data.iter().rev() { - if let Some(data) = container.get::>() { - return Some(Data::clone(&data)); + if let Some(data) = container.get::() { + return Some(data); } } @@ -595,6 +593,28 @@ mod tests { let resp = srv.call(req).await.unwrap(); assert_eq!(resp.status(), http::StatusCode::NOT_FOUND); } + + #[actix_rt::test] + async fn test_service_data() { + let mut srv = init_service( + App::new() + .data(42u32) + .service(web::service("/test").name("test").finish( + |req: ServiceRequest| { + assert_eq!( + req.app_data::>().unwrap().as_ref(), + &42 + ); + ok(req.into_response(HttpResponse::Ok().finish())) + }, + )), + ) + .await; + let req = TestRequest::with_uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), http::StatusCode::OK); + } + #[test] fn test_fmt_debug() { let req = TestRequest::get() From 160995b8d49b8f1e556e4944617c9982f1ac2ad8 Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Mon, 10 Aug 2020 04:49:43 +0800 Subject: [PATCH 4/4] fix awc pool leak (#1626) --- actix-http/CHANGES.md | 6 +- actix-http/src/client/pool.rs | 103 ++++++++++++++++++---------------- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index b2b816d54..8d44ea6b9 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,9 +1,13 @@ # Changes ## Unreleased - 2020-xx-xx +### Fixed +* Memory leak of `client::pool::ConnectorPoolSupport`. [#1626] + +[#1626]: https://github.com/actix/actix-web/pull/1626 -## 2.0.0-beta.2 - 2020-07-21 +## [2.0.0-beta.2] - 2020-07-21 ### Fixed * Potential UB in h1 decoder using uninitialized memory. [#1614] diff --git a/actix-http/src/client/pool.rs b/actix-http/src/client/pool.rs index f2c5b0410..013a79671 100644 --- a/actix-http/src/client/pool.rs +++ b/actix-http/src/client/pool.rs @@ -2,7 +2,7 @@ use std::cell::RefCell; use std::collections::VecDeque; use std::future::Future; use std::pin::Pin; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::task::{Context, Poll}; use std::time::{Duration, Instant}; @@ -65,8 +65,8 @@ where // start support future actix_rt::spawn(ConnectorPoolSupport { - connector: connector_rc.clone(), - inner: Rc::downgrade(&inner_rc), + connector: Rc::clone(&connector_rc), + inner: Rc::clone(&inner_rc), }); ConnectionPool(connector_rc, inner_rc) @@ -82,6 +82,13 @@ where } } +impl Drop for ConnectionPool { + fn drop(&mut self) { + // wake up the ConnectorPoolSupport when dropping so it can exit properly. + self.1.borrow().waker.wake(); + } +} + impl Service for ConnectionPool where Io: AsyncRead + AsyncWrite + Unpin + 'static, @@ -421,7 +428,7 @@ where Io: AsyncRead + AsyncWrite + Unpin + 'static, { connector: T, - inner: Weak>>, + inner: Rc>>, } impl Future for ConnectorPoolSupport @@ -435,55 +442,57 @@ where fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.project(); - if let Some(this_inner) = this.inner.upgrade() { - let mut inner = this_inner.as_ref().borrow_mut(); - inner.waker.register(cx.waker()); + if Rc::strong_count(this.inner) == 1 { + // If we are last copy of Inner it means the ConnectionPool is already gone + // and we are safe to exit. + return Poll::Ready(()); + } - // check waiters - loop { - let (key, token) = { - if let Some((key, token)) = inner.waiters_queue.get_index(0) { - (key.clone(), *token) - } else { - break; - } - }; - if inner.waiters.get(token).unwrap().is_none() { - continue; - } + let mut inner = this.inner.borrow_mut(); + inner.waker.register(cx.waker()); - match inner.acquire(&key, cx) { - Acquire::NotAvailable => break, - Acquire::Acquired(io, created) => { - let tx = inner.waiters.get_mut(token).unwrap().take().unwrap().1; - if let Err(conn) = tx.send(Ok(IoConnection::new( - io, - created, - Some(Acquired(key.clone(), Some(this_inner.clone()))), - ))) { - let (io, created) = conn.unwrap().into_inner(); - inner.release_conn(&key, io, created); - } - } - Acquire::Available => { - let (connect, tx) = - inner.waiters.get_mut(token).unwrap().take().unwrap(); - OpenWaitingConnection::spawn( - key.clone(), - tx, - this_inner.clone(), - this.connector.call(connect), - inner.config.clone(), - ); - } + // check waiters + loop { + let (key, token) = { + if let Some((key, token)) = inner.waiters_queue.get_index(0) { + (key.clone(), *token) + } else { + break; } - let _ = inner.waiters_queue.swap_remove_index(0); + }; + if inner.waiters.get(token).unwrap().is_none() { + continue; } - Poll::Pending - } else { - Poll::Ready(()) + match inner.acquire(&key, cx) { + Acquire::NotAvailable => break, + Acquire::Acquired(io, created) => { + let tx = inner.waiters.get_mut(token).unwrap().take().unwrap().1; + if let Err(conn) = tx.send(Ok(IoConnection::new( + io, + created, + Some(Acquired(key.clone(), Some(this.inner.clone()))), + ))) { + let (io, created) = conn.unwrap().into_inner(); + inner.release_conn(&key, io, created); + } + } + Acquire::Available => { + let (connect, tx) = + inner.waiters.get_mut(token).unwrap().take().unwrap(); + OpenWaitingConnection::spawn( + key.clone(), + tx, + this.inner.clone(), + this.connector.call(connect), + inner.config.clone(), + ); + } + } + let _ = inner.waiters_queue.swap_remove_index(0); } + + Poll::Pending } }