Merge pull request DMDcoin#269 from DMDcoin/0.11

release 0.11.8

Author: SurfingNerd

CHANGELOG.md

@@ -1,3 +1,6 @@
+## Diamond Node Software 3.3.5-hbbft-0.11.8
+-  [deregister_session_stream can cause deadlocks](https://github.com/DMDcoin/diamond-node/issues/267)
+
 ## Diamond Node Software 3.3.5-hbbft-0.11.7
 - [Handshake and Session Management improvements](https://github.com/DMDcoin/diamond-node/issues/262)
 - [Reliable Message Broadcast Protocol: message_cache of SyncProtocolHandler does not get cleaned up](https://github.com/DMDcoin/diamond-node/issues/261)

Cargo.lock

@@ -2,7 +2,7 @@
 description = "Diamond Node"
 name = "diamond-node"
 # NOTE Make sure to update util/version/Cargo.toml as well
-version = "3.3.5-hbbft-0.11.7"
+version = "3.3.5-hbbft-0.11.8"
 license = "GPL-3.0"
 authors = [
 	"bit.diamonds developers",

Cargo.toml

@@ -1613,7 +1613,13 @@ impl IoHandler<NetworkIoMessage> for Host {
         event_loop: &mut EventLoop<IoManager<NetworkIoMessage>>,
     ) {
         match stream {
-            FIRST_HANDSHAKE.. => {
+            FIRST_HANDSHAKE..FIRST_SESSION => {
+                let _handhake_lock = self.handshake_lock.lock(); // we do not allow new handshakes to get processed during deregistering a stream.
+                self.sessions
+                    .deregister_handshake_stream(stream, event_loop);
+            }
+            FIRST_SESSION..=LAST_SESSION => {
+                let _handhake_lock = self.handshake_lock.lock(); // since finalizing handshakes is the only way to promot a handshake to a session, we also block handshakes here.
                 self.sessions.deregister_session_stream(stream, event_loop);
             }
             DISCOVERY => (),

crates/net/network-devp2p/src/host.rs

@@ -6,7 +6,7 @@ use std::{collections::BTreeMap, sync::Arc, time::Duration};
 
 use crate::{io::*, node_table::*, session::Session};
 use network::{Error, ErrorKind, NetworkIoMessage, PeerId};
-use parking_lot::{Mutex, RwLock, RwLockReadGuard, RwLockUpgradableReadGuard};
+use parking_lot::{Mutex, RwLock, RwLockReadGuard};
 
 pub type SharedSession = Arc<Mutex<Session>>;
 
@@ -430,28 +430,53 @@ impl SessionContainer {
         return None;
     }
 
+    pub(crate) fn deregister_handshake_stream<Host: mio::deprecated::Handler>(
+        &self,
+        stream: usize,
+        event_loop: &mut mio::deprecated::EventLoop<Host>,
+    ) {
+        if stream < self.first_handshake || stream >= self.last_handshake {
+            warn!(target: "network", "Tried to deregister handshake stream {} but it is out of range.", stream);
+            return;
+        }
+
+        if let Some(connection) = self.get_handshake(stream) {
+            let c = connection.lock();
+            if c.expired() {
+                // make sure it is the same connection that the event was generated for
+                c.deregister_socket(event_loop)
+                    .expect("Error deregistering socket");
+                drop(c);
+
+                self.handshakes.write().remove(&stream);
+                //RwLockUpgradableReadGuard::<'_, parking_lot::RawRwLock, BTreeMap<usize, Arc<parking_lot::lock_api::Mutex<parking_lot::RawMutex, Session>>>>::upgrade(connections).remove(&stream);
+            } else {
+                debug!(target: "network", "Tried to deregister session stream {} but it is not expired.", stream);
+            }
+        } else {
+            debug!(target: "network", "Tried to deregister session stream {} but it does not exist.", stream);
+        }
+    }
+
     pub(crate) fn deregister_session_stream<Host: mio::deprecated::Handler>(
         &self,
         stream: usize,
 
         event_loop: &mut mio::deprecated::EventLoop<Host>,
     ) {
-        let connections = if stream < self.last_handshake {
-            self.handshakes.upgradable_read()
-        } else {
-            self.sessions.upgradable_read()
-        };
+        if stream < self.last_handshake + 1 {
+            warn!(target: "network", "Tried to deregister session stream {} but it is out of range.", stream);
+            return;
+        }
 
-        if let Some(connection) = connections.get(&stream).cloned() {
+        if let Some(connection) = self.get_session(stream) {
             let c = connection.lock();
             if c.expired() {
                 // make sure it is the same connection that the event was generated for
                 c.deregister_socket(event_loop)
                     .expect("Error deregistering socket");
                 drop(c);
-                let mut connections_write = RwLockUpgradableReadGuard::upgrade(connections);
-                connections_write.remove(&stream);
-                //RwLockUpgradableReadGuard::<'_, parking_lot::RawRwLock, BTreeMap<usize, Arc<parking_lot::lock_api::Mutex<parking_lot::RawMutex, Session>>>>::upgrade(connections).remove(&stream);
+                self.sessions.write().remove(&stream);
             } else {
                 debug!(target: "network", "Tried to deregister session stream {} but it is not expired.", stream);
             }

crates/net/network-devp2p/src/session_container.rs

@@ -1,7 +1,7 @@
 [package]
 name = "parity-version"
 # NOTE: this value is used for OpenEthereum version string (via env CARGO_PKG_VERSION)
-version = "3.3.5-hbbft-0.11.7"
+version = "3.3.5-hbbft-0.11.8"
 authors = [
 	"bit.diamonds developers",
 	"OpenEthereum developers",