feat(swarm): deprecate KeepAlive::Until

In preparation for removing this variant, we are issuing a deprecation notice. The linked issue describes why and guides users in migrating away from it. This deprecation is backwards-compatible and allows us to remove the variant in the next breaking release. We haven't migrated all internal protocols yet but that isn't strictly necessary to issue the deprecation. Related: #3844. Pull-Request: #4656.
libp2p · Oct 16, 2023 · 7c6f75e · 7c6f75e
1 parent e545805
commit 7c6f75e
Show file tree

Hide file tree

Showing 17 changed files with 76 additions and 24 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -96,10 +96,10 @@ libp2p-ping = { version = "0.43.1", path = "protocols/ping" }
 libp2p-plaintext = { version = "0.40.1", path = "transports/plaintext" }
 libp2p-pnet = { version = "0.23.0", path = "transports/pnet" }
 libp2p-quic = { version = "0.9.3", path = "transports/quic" }
-libp2p-relay = { version = "0.16.1", path = "protocols/relay" }
+libp2p-relay = { version = "0.16.2", path = "protocols/relay" }
 libp2p-rendezvous = { version = "0.13.1", path = "protocols/rendezvous" }
 libp2p-upnp = { version = "0.1.1", path = "protocols/upnp" }
-libp2p-request-response = { version = "0.25.1", path = "protocols/request-response" }
+libp2p-request-response = { version = "0.25.2", path = "protocols/request-response" }
 libp2p-server = { version = "0.12.3", path = "misc/server" }
 libp2p-swarm = { version = "0.43.6", path = "swarm" }
 libp2p-swarm-derive = { version = "0.33.0", path = "swarm-derive" }

diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md
@@ -5,6 +5,12 @@
 
 [PR 4648]: (https://github.com/libp2p/rust-libp2p/pull/4648)
 
+<!-- Internal changes
+
+- Allow deprecated usage of `KeepAlive::Until`
+
+-->
+
 ## 0.45.1
 
 - Add getter function to obtain `TopicScoreParams`.

diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs
@@ -444,6 +444,7 @@ impl ConnectionHandler for Handler {
                     return KeepAlive::Yes;
                 }
 
+                #[allow(deprecated)]
                 KeepAlive::Until(handler.last_io_activity + handler.idle_timeout)
             }
             Handler::Disabled(_) => KeepAlive::No,

diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md
@@ -8,6 +8,12 @@
 
 [PR 4547]: https://github.com/libp2p/rust-libp2p/pull/4547
 
+<!-- Internal changes
+
+- Allow deprecated usage of `KeepAlive::Until`
+
+-->
+
 ## 0.44.5
 - Migrate to `quick-protobuf-codec` crate for codec logic.
   See [PR 4501].

diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs
@@ -484,6 +484,7 @@ impl Handler {
             }
         }
 
+        #[allow(deprecated)]
         let keep_alive = KeepAlive::Until(Instant::now() + idle_timeout);
 
         Handler {
@@ -769,13 +770,17 @@ impl ConnectionHandler for Handler {
         }
 
         let no_streams = self.outbound_substreams.is_empty() && self.inbound_substreams.is_empty();
-        self.keep_alive = match (no_streams, self.keep_alive) {
-            // No open streams. Preserve the existing idle timeout.
-            (true, k @ KeepAlive::Until(_)) => k,
-            // No open streams. Set idle timeout.
-            (true, _) => KeepAlive::Until(Instant::now() + self.idle_timeout),
-            // Keep alive for open streams.
-            (false, _) => KeepAlive::Yes,
+
+        self.keep_alive = {
+            #[allow(deprecated)]
+            match (no_streams, self.keep_alive) {
+                // No open streams. Preserve the existing idle timeout.
+                (true, k @ KeepAlive::Until(_)) => k,
+                // No open streams. Set idle timeout.
+                (true, _) => KeepAlive::Until(Instant::now() + self.idle_timeout),
+                // Keep alive for open streams.
+                (false, _) => KeepAlive::Yes,
+            }
         };
 
         Poll::Pending

diff --git a/protocols/relay/CHANGELOG.md b/protocols/relay/CHANGELOG.md
@@ -1,3 +1,11 @@
+## 0.16.2 - unreleased
+
+<!-- Internal changes
+
+- Allow deprecated usage of `KeepAlive::Until`
+
+-->
+
 ## 0.16.1
 
 - Export `RateLimiter` type.

diff --git a/protocols/relay/Cargo.toml b/protocols/relay/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-relay"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Communications relaying for libp2p"
-version = "0.16.1"
+version = "0.16.2"
 authors = ["Parity Technologies <admin@parity.io>", "Max Inden <mail@max-inden.de>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

diff --git a/protocols/relay/src/behaviour/handler.rs b/protocols/relay/src/behaviour/handler.rs
@@ -883,6 +883,7 @@ impl ConnectionHandler for Handler {
             && self.circuits.is_empty()
             && self.active_reservation.is_none()
         {
+            #[allow(deprecated)]
             match self.keep_alive {
                 KeepAlive::Yes => {
                     self.keep_alive = KeepAlive::Until(Instant::now() + Duration::from_secs(10));

diff --git a/protocols/relay/src/priv_client/handler.rs b/protocols/relay/src/priv_client/handler.rs
@@ -489,21 +489,24 @@ impl ConnectionHandler for Handler {
         }
 
         // Update keep-alive handling.
-        if matches!(self.reservation, Reservation::None)
-            && self.alive_lend_out_substreams.is_empty()
-            && self.circuit_deny_futs.is_empty()
+        #[allow(deprecated)]
         {
-            match self.keep_alive {
-                KeepAlive::Yes => {
-                    self.keep_alive = KeepAlive::Until(Instant::now() + Duration::from_secs(10));
+            if matches!(self.reservation, Reservation::None)
+                && self.alive_lend_out_substreams.is_empty()
+                && self.circuit_deny_futs.is_empty()
+            {
+                match self.keep_alive {
+                    KeepAlive::Yes => {
+                        self.keep_alive =
+                            KeepAlive::Until(Instant::now() + Duration::from_secs(10));
+                    }
+                    KeepAlive::Until(_) => {}
+                    KeepAlive::No => panic!("Handler never sets KeepAlive::No."),
                 }
-                KeepAlive::Until(_) => {}
-                KeepAlive::No => panic!("Handler never sets KeepAlive::No."),
+            } else {
+                self.keep_alive = KeepAlive::Yes;
             }
-        } else {
-            self.keep_alive = KeepAlive::Yes;
         }
-
         Poll::Pending
     }
 

diff --git a/protocols/request-response/CHANGELOG.md b/protocols/request-response/CHANGELOG.md
@@ -1,3 +1,11 @@
+## 0.25.2 - unreleased
+
+<!-- Internal changes
+
+- Allow deprecated usage of `KeepAlive::Until`
+
+-->
+
 ## 0.25.1
 
 - Replace unmaintained `serde_cbor` dependency with `cbor4ii`.

diff --git a/protocols/request-response/Cargo.toml b/protocols/request-response/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-request-response"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Generic Request/Response Protocols"
-version = "0.25.1"
+version = "0.25.2"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

diff --git a/protocols/request-response/src/handler.rs b/protocols/request-response/src/handler.rs
@@ -336,6 +336,7 @@ where
             self.outbound.shrink_to_fit();
         }
 
+        #[allow(deprecated)]
         if self.inbound.is_empty() && self.keep_alive.is_yes() {
             // No new inbound or outbound requests. However, we may just have
             // started the latest inbound or outbound upgrade(s), so make sure

diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md
@@ -6,6 +6,10 @@
   See [PR 4120].
 - Make the `Debug` implementation of `StreamProtocol` more concise.
   See [PR 4631](https://github.com/libp2p/rust-libp2p/pull/4631).
+- Deprecate `KeepAlive::Until`.
+  Individual protocols should not keep connections alive for longer than necessary.
+  Users should use `swarm::Config::idle_connection_timeout` instead.
+  See [PR 4656](https://github.com/libp2p/rust-libp2p/pull/4656).
 
 [PR 4120]: https://github.com/libp2p/rust-libp2p/pull/4120
 

diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs
@@ -347,6 +347,7 @@ where
             // Ask the handler whether it wants the connection (and the handler itself)
             // to be kept alive, which determines the planned shutdown, if any.
             let keep_alive = handler.connection_keep_alive();
+            #[allow(deprecated)]
             match (&mut *shutdown, keep_alive) {
                 (Shutdown::Later(timer, deadline), KeepAlive::Until(t)) => {
                     if *deadline != t {
@@ -1005,6 +1006,7 @@ mod tests {
         }
 
         fn connection_keep_alive(&self) -> KeepAlive {
+            #[allow(deprecated)]
             KeepAlive::Until(self.until)
         }
 

diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs
@@ -728,6 +728,9 @@ where
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum KeepAlive {
     /// If nothing new happens, the connection should be closed at the given `Instant`.
+    #[deprecated(
+        note = "Use `swarm::Config::with_idle_connection_timeout` instead. See <https://github.com/libp2p/rust-libp2p/issues/3844> for details."
+    )]
     Until(Instant),
     /// Keep the connection alive.
     Yes,
@@ -748,6 +751,7 @@ impl PartialOrd for KeepAlive {
     }
 }
 
+#[allow(deprecated)]
 impl Ord for KeepAlive {
     fn cmp(&self, other: &KeepAlive) -> Ordering {
         use self::KeepAlive::*;

diff --git a/swarm/src/handler/one_shot.rs b/swarm/src/handler/one_shot.rs
@@ -176,6 +176,7 @@ where
         } else {
             self.dial_queue.shrink_to_fit();
 
+            #[allow(deprecated)]
             if self.dial_negotiated == 0 && self.keep_alive.is_yes() {
                 self.keep_alive = KeepAlive::Until(Instant::now() + self.config.keep_alive_timeout);
             }
@@ -199,6 +200,7 @@ where
                 ..
             }) => {
                 // If we're shutting down the connection for inactivity, reset the timeout.
+                #[allow(deprecated)]
                 if !self.keep_alive.is_yes() {
                     self.keep_alive =
                         KeepAlive::Until(Instant::now() + self.config.keep_alive_timeout);
@@ -258,6 +260,7 @@ mod tests {
     use void::Void;
 
     #[test]
+    #[allow(deprecated)]
     fn do_not_keep_idle_connection_alive() {
         let mut handler: OneShotHandler<_, DeniedUpgrade, Void> = OneShotHandler::new(
             SubstreamProtocol::new(DeniedUpgrade {}, ()),