From 7b51b1e88d1702d4ecaaea8bd12272469ebd774e Mon Sep 17 00:00:00 2001 From: catbref Date: Tue, 2 Jul 2019 12:14:32 +0100 Subject: [PATCH] Improve Network.shutdown() and logging in Peer. Network.shutdown() called Peer.shutdown() on each Peer while holding synchronization lock on this.connectedPeers. This would cause a problem during Peer.shutdown() as some other reachable code would also want synchronized access to this.connectedPeers. Typical symptoms would be log entries like: 2019-07-02 11:13:05 DEBUG Peer:512 - Message processor for peer 192.144.182.73:9889 failed to terminate Eventually Network.shutdown() would exit, releasing synchronization lock and awaking stuck Peer threads, which could then try to access repository (now closed) causing further log spam. Now it uses Network.getConnectedPeers to return duplicated List, minimizing lock time on this.connectedPeers. Also made Peer main thread logging more informative when a IOException occurs, as most situations are harmless EOF or connection reset by peer. --- src/main/java/org/qora/network/Network.java | 6 ++---- src/main/java/org/qora/network/Peer.java | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/qora/network/Network.java b/src/main/java/org/qora/network/Network.java index 0916422a..b5321812 100644 --- a/src/main/java/org/qora/network/Network.java +++ b/src/main/java/org/qora/network/Network.java @@ -963,10 +963,8 @@ public class Network extends Thread { } // Close all peer connections - synchronized (this.connectedPeers) { - for (Peer peer : this.connectedPeers) - peer.shutdown(); - } + for (Peer peer : this.getConnectedPeers()) + peer.shutdown(); } } diff --git a/src/main/java/org/qora/network/Peer.java b/src/main/java/org/qora/network/Peer.java index 61bb5d47..69aec170 100644 --- a/src/main/java/org/qora/network/Peer.java +++ b/src/main/java/org/qora/network/Peer.java @@ -1,6 +1,7 @@ package org.qora.network; import java.io.DataInputStream; +import java.io.EOFException; import java.io.IOException; import java.io.OutputStream; import java.net.InetAddress; @@ -369,10 +370,20 @@ public class Peer extends Thread { } catch (SocketTimeoutException e) { this.disconnect("timeout"); } catch (IOException e) { - if (isStopping) + if (isStopping) { + // If isStopping is true then our shutdown() has already been called, so no need to call it again LOGGER.debug(String.format("Peer %s stopping...", this)); - else + return; + } + + // More informative logging + if (e instanceof EOFException) { + this.disconnect("EOF"); + } else if (e.getMessage().contains("onnection reset")) { // Can't import/rely on sun.net.ConnectionResetException + this.disconnect("Connection reset"); + } else { this.disconnect("I/O error"); + } } finally { Thread.currentThread().setName("disconnected peer"); }