From a55946cf053a0e991b5f9b4b6a2bb34f35507505 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 16:29:26 +0800 Subject: [PATCH 01/13] refactor: use `matches` macro to flatten code --- src/proto/h1/conn.rs | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 74571baa15..8067c1b9c8 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -155,19 +155,15 @@ where } pub(crate) fn can_read_head(&self) -> bool { - match self.state.reading { - Reading::Init => { - if T::should_read_first() { - true - } else { - match self.state.writing { - Writing::Init => false, - _ => true, - } - } - } - _ => false, + if !matches!(self.state.reading, Reading::Init) { + return false; + } + + if T::should_read_first() { + return true; } + + !matches!(self.state.writing, Writing::Init) } pub(crate) fn can_read_body(&self) -> bool { @@ -367,10 +363,7 @@ where } fn is_mid_message(&self) -> bool { - match (&self.state.reading, &self.state.writing) { - (&Reading::Init, &Writing::Init) => false, - _ => true, - } + !matches!((self.state.reading, self.state.writing), (Reading::Init, Writing::Init)) } // This will check to make sure the io object read is empty. From aeda2303e7e1d6347ab31dd310a087925669e941 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 16:45:25 +0800 Subject: [PATCH 02/13] refactor: prefer `matches` over explicit matching --- src/proto/h1/conn.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 8067c1b9c8..a57ec2a9e7 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -439,16 +439,12 @@ where // determined we couldn't keep reading until we knew how writing // would finish. - match self.state.reading { - Reading::Continue(..) | Reading::Body(..) | Reading::KeepAlive | Reading::Closed => { - return - } - Reading::Init => (), - }; + if !matches!(self.state.reading, Reading::Init) { + return; + } - match self.state.writing { - Writing::Body(..) => return, - Writing::Init | Writing::KeepAlive | Writing::Closed => (), + if matches!(self.state.writing, Writing::Body(..)) { + return; } if !self.io.is_read_blocked() { From 5865910064c5e38e3f182a6fbe8aa76d99d3e2c1 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 16:53:34 +0800 Subject: [PATCH 03/13] refactor: reuse `can_write_body` method --- src/proto/h1/conn.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index a57ec2a9e7..bb5c49a36e 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -443,7 +443,7 @@ where return; } - if matches!(self.state.writing, Writing::Body(..)) { + if self.can_write_body() { return; } @@ -494,10 +494,7 @@ where } pub(crate) fn can_write_body(&self) -> bool { - match self.state.writing { - Writing::Body(..) => true, - Writing::Init | Writing::KeepAlive | Writing::Closed => false, - } + matches!(self.state.writing, Writing::Body(..)) } pub(crate) fn can_buffer_body(&self) -> bool { From 38601c3d09b1b39bef29b287951e687936f40fd0 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 16:58:19 +0800 Subject: [PATCH 04/13] refactor: use `matches` over `if`-`let` --- src/proto/h1/conn.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index bb5c49a36e..976447044c 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -482,11 +482,10 @@ where } pub(crate) fn can_write_head(&self) -> bool { - if !T::should_read_first() { - if let Reading::Closed = self.state.reading { - return false; - } + if !T::should_read_first() && matches!(self.state.reading, Reading::Closed) { + return false; } + match self.state.writing { Writing::Init => self.io.can_headers_buf(), _ => false, From 83691fcc45db5b1e9ecf4ecc28442136c5324259 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:00:52 +0800 Subject: [PATCH 05/13] refactor: nested `if`-`else` as early return --- src/proto/h1/conn.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 976447044c..910fdb54a6 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -626,15 +626,15 @@ where Writing::Body(ref mut encoder) => { self.io.buffer(encoder.encode(chunk)); - if encoder.is_eof() { - if encoder.is_last() { - Writing::Closed - } else { - Writing::KeepAlive - } - } else { + if !encoder.is_eof() { return; } + + if encoder.is_last() { + Writing::Closed + } else { + Writing::KeepAlive + } } _ => unreachable!("write_body invalid state: {:?}", self.state.writing), }; From 530ebc2b2d504bcce9c4eb86e1f56aa493d515f4 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:09:37 +0800 Subject: [PATCH 06/13] refactor: move inner `match` logic outside --- src/proto/h1/conn.rs | 45 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 910fdb54a6..95e2b03b79 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -665,32 +665,31 @@ where pub(crate) fn end_body(&mut self) -> crate::Result<()> { debug_assert!(self.can_write_body()); - let mut res = Ok(()); - let state = match self.state.writing { - Writing::Body(ref mut encoder) => { - // end of stream, that means we should try to eof - match encoder.end() { - Ok(end) => { - if let Some(end) = end { - self.io.buffer(end); - } - if encoder.is_last() || encoder.is_close_delimited() { - Writing::Closed - } else { - Writing::KeepAlive - } - } - Err(not_eof) => { - res = Err(crate::Error::new_body_write_aborted().with(not_eof)); - Writing::Closed - } - } - } + let encoder = match self.state.writing { + Writing::Body(ref mut enc) => enc, _ => return Ok(()), }; - self.state.writing = state; - res + // end of stream, that means we should try to eof + match encoder.end() { + Ok(end) => { + if let Some(end) = end { + self.io.buffer(end); + } + + self.state.writing = if encoder.is_last() || encoder.is_close_delimited() { + Writing::Closed + } else { + Writing::KeepAlive + }; + + Ok(()) + } + Err(not_eof) => { + self.state.writing = Writing::Closed; + Err(crate::Error::new_body_write_aborted().with(not_eof)) + } + }; } // When we get a parse error, depending on what side we are, we might be able From e1cd9c59001f01dac17fc5d536fbd75c993095c8 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:14:46 +0800 Subject: [PATCH 07/13] refactor: unneeded `return` in `match` --- src/proto/h1/conn.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 95e2b03b79..2596fe1e63 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -363,7 +363,10 @@ where } fn is_mid_message(&self) -> bool { - !matches!((self.state.reading, self.state.writing), (Reading::Init, Writing::Init)) + !matches!( + (self.state.reading, self.state.writing), + (Reading::Init, Writing::Init) + ) } // This will check to make sure the io object read is empty. @@ -742,10 +745,7 @@ where // If still in Reading::Body, just give up match self.state.reading { - Reading::Init | Reading::KeepAlive => { - trace!("body drained"); - return; - } + Reading::Init | Reading::KeepAlive => trace!("body drained"), _ => self.close_read(), } } From 8530a593797287bdfc905db8ed35fbf807ea5a48 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:18:18 +0800 Subject: [PATCH 08/13] refactor: remove unneeded reference matching --- src/proto/h1/conn.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 2596fe1e63..467f4b994e 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -960,8 +960,8 @@ impl State { } fn try_keep_alive(&mut self) { - match (&self.reading, &self.writing) { - (&Reading::KeepAlive, &Writing::KeepAlive) => { + match (self.reading, self.writing) { + (Reading::KeepAlive, Writing::KeepAlive) => { if let KA::Busy = self.keep_alive.status() { self.idle::(); } else { @@ -973,7 +973,7 @@ impl State { self.close(); } } - (&Reading::Closed, &Writing::KeepAlive) | (&Reading::KeepAlive, &Writing::Closed) => { + (Reading::Closed, Writing::KeepAlive) | (Reading::KeepAlive, Writing::Closed) => { self.close() } _ => (), From 36b62f287e96f1ecc8ecfe587e180e727b7122cb Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:20:35 +0800 Subject: [PATCH 09/13] refactor: use early returns for idle check --- src/proto/h1/conn.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 467f4b994e..fa7cf0abb3 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -996,20 +996,22 @@ impl State { self.method = None; self.keep_alive.idle(); - if self.is_idle() { - self.reading = Reading::Init; - self.writing = Writing::Init; - - // !T::should_read_first() means Client. - // - // If Client connection has just gone idle, the Dispatcher - // should try the poll loop one more time, so as to poll the - // pending requests stream. - if !T::should_read_first() { - self.notify_read = true; - } - } else { + + if !self.is_idle() { self.close(); + return; + } + + self.reading = Reading::Init; + self.writing = Writing::Init; + + // !T::should_read_first() means Client. + // + // If Client connection has just gone idle, the Dispatcher + // should try the poll loop one more time, so as to poll the + // pending requests stream. + if !T::should_read_first() { + self.notify_read = true; } } From da23775562d4af6dffefd289eb43be39b6c9de6f Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:21:22 +0800 Subject: [PATCH 10/13] refactor: use `matches` macro for Boolean `match` --- src/proto/h1/conn.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index fa7cf0abb3..74244c2c02 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -1016,25 +1016,15 @@ impl State { } fn is_idle(&self) -> bool { - if let KA::Idle = self.keep_alive.status() { - true - } else { - false - } + matches!(self.keep_alive.status(), KA::Idle) } fn is_read_closed(&self) -> bool { - match self.reading { - Reading::Closed => true, - _ => false, - } + matches!(self.reading, Reading::Closed) } fn is_write_closed(&self) -> bool { - match self.writing { - Writing::Closed => true, - _ => false, - } + matches!(self.writing, Writing::Closed) } fn prepare_upgrade(&mut self) -> crate::upgrade::OnUpgrade { From 8817396048ce493ac7d78471b61882d25adee730 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:39:03 +0800 Subject: [PATCH 11/13] fix: remove accidentally introduced semicolon --- src/proto/h1/conn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 74244c2c02..cff69bc4c2 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -692,7 +692,7 @@ where self.state.writing = Writing::Closed; Err(crate::Error::new_body_write_aborted().with(not_eof)) } - }; + } } // When we get a parse error, depending on what side we are, we might be able From 7aab0f7c2adc46cdd811f9fcd5c30e7cca276225 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Sun, 24 Apr 2022 17:47:07 +0800 Subject: [PATCH 12/13] fix: revert removal of references It turns out `Copy` was not implemented. Oops! --- src/proto/h1/conn.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index cff69bc4c2..2a4c6fe032 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -364,8 +364,8 @@ where fn is_mid_message(&self) -> bool { !matches!( - (self.state.reading, self.state.writing), - (Reading::Init, Writing::Init) + (&self.state.reading, &self.state.writing), + (&Reading::Init, &Writing::Init) ) } @@ -960,8 +960,8 @@ impl State { } fn try_keep_alive(&mut self) { - match (self.reading, self.writing) { - (Reading::KeepAlive, Writing::KeepAlive) => { + match (&self.reading, &self.writing) { + (&Reading::KeepAlive, &Writing::KeepAlive) => { if let KA::Busy = self.keep_alive.status() { self.idle::(); } else { @@ -973,7 +973,7 @@ impl State { self.close(); } } - (Reading::Closed, Writing::KeepAlive) | (Reading::KeepAlive, Writing::Closed) => { + (&Reading::Closed, &Writing::KeepAlive) | (&Reading::KeepAlive, &Writing::Closed) => { self.close() } _ => (), From e0f639cb8e42e58afaddaab2dc9b14246616763d Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Wed, 27 Apr 2022 00:03:47 +0800 Subject: [PATCH 13/13] revert: use explicit pattern-matching for some cases --- src/proto/h1/conn.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 2a4c6fe032..7d7c3fd2d3 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -442,12 +442,16 @@ where // determined we couldn't keep reading until we knew how writing // would finish. - if !matches!(self.state.reading, Reading::Init) { - return; - } + match self.state.reading { + Reading::Continue(..) | Reading::Body(..) | Reading::KeepAlive | Reading::Closed => { + return + } + Reading::Init => (), + }; - if self.can_write_body() { - return; + match self.state.writing { + Writing::Body(..) => return, + Writing::Init | Writing::KeepAlive | Writing::Closed => (), } if !self.io.is_read_blocked() { @@ -496,7 +500,10 @@ where } pub(crate) fn can_write_body(&self) -> bool { - matches!(self.state.writing, Writing::Body(..)) + match self.state.writing { + Writing::Body(..) => true, + Writing::Init | Writing::KeepAlive | Writing::Closed => false, + } } pub(crate) fn can_buffer_body(&self) -> bool {