Networking improvements

Fix issue where sometimes the channelSelector.select(1000) would
block processing of queued messages.

Improve support with older v1 peers.

src/main/java/org/qora/controller/Synchronizer.java

@@ -1,6 +1,5 @@
 package org.qora.controller;
 
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.locks.ReentrantLock;

src/main/java/org/qora/network/Network.java

@@ -288,11 +288,6 @@ public class Network extends Thread {
 		protected Task produceTask(boolean canBlock) throws InterruptedException {
 			Task task;
 
-			// Only this method can block to reduce CPU spin
-			task = maybeProduceChannelTask(canBlock);
-			if (task != null)
-				return task;
-
 			task = maybeProducePeerMessageTask();
 			if (task != null)
 				return task;
@@ -309,6 +304,11 @@ public class Network extends Thread {
 			if (task != null)
 				return task;
 
+			// Only this method can block to reduce CPU spin
+			task = maybeProduceChannelTask(canBlock);
+			if (task != null)
+				return task;
+
 			// Really nothing to do
 			return null;
 		}
@@ -688,6 +688,10 @@ public class Network extends Thread {
 
 	// Peer callbacks
 
+	/* package */ void wakeupChannelSelector() {
+		this.channelSelector.wakeup();
+	}
+
 	/** Called when Peer's thread has setup and is ready to process messages */
 	public void onPeerReady(Peer peer) {
 		this.onMessage(peer, null);
@@ -719,14 +723,18 @@ public class Network extends Thread {
 
 		Handshake handshakeStatus = peer.getHandshakeStatus();
 		if (handshakeStatus != Handshake.COMPLETED) {
+			try {
 				// Still handshaking
+				LOGGER.trace(() -> String.format("Handshake status %s, message %s from peer %s", handshakeStatus.name(), (message != null ? message.getType().name() : "null"), peer));
 
-			// Check message type is as expected
+				// v1 nodes are keen on sending PINGs early. Send to back of queue so we'll process right after handshake
-			if (handshakeStatus.expectedMessageType != null && message.getType() != handshakeStatus.expectedMessageType) {
+				if (message != null && message.getType() == MessageType.PING) {
-				// v1 nodes are keen on sending PINGs early. Discard as we'll send a PING right after handshake
+					peer.queueMessage(message);
-				if (message.getType() == MessageType.PING)
 					return;
+				}
 
+				// Check message type is as expected
+				if (handshakeStatus.expectedMessageType != null && message.getType() != handshakeStatus.expectedMessageType) {
 					LOGGER.debug(String.format("Unexpected %s message from %s, expected %s", message.getType().name(), peer, handshakeStatus.expectedMessageType));
 					peer.disconnect("unexpected message");
 					return;
@@ -754,6 +762,9 @@ public class Network extends Thread {
 					this.onHandshakeCompleted(peer);
 
 				return;
+			} finally {
+				peer.resetHandshakeMessagePending();
+			}
 		}
 
 		// Should be non-handshaking messages from now on

src/main/java/org/qora/network/Peer.java

@@ -59,50 +59,51 @@ public class Peer {
 	private InetSocketAddress resolvedAddress = null;
 	/** True if remote address is loopback/link-local/site-local, false otherwise. */
 	private boolean isLocal;
-	private ByteBuffer byteBuffer;
+	private volatile ByteBuffer byteBuffer;
 	private Map<Integer, BlockingQueue<Message>> replyQueues;
 	private LinkedBlockingQueue<Message> pendingMessages;
 
 	/** True if we created connection to peer, false if we accepted incoming connection from peer. */
 	private final boolean isOutbound;
 	/** Numeric protocol version, typically 1 or 2. */
-	private Integer version;
+	private volatile Integer version;
-	private byte[] peerId;
+	private volatile byte[] peerId;
 
-	private Handshake handshakeStatus = Handshake.STARTED;
+	private volatile Handshake handshakeStatus = Handshake.STARTED;
+	private volatile boolean handshakeMessagePending = false;
 
-	private byte[] pendingPeerId;
+	private volatile byte[] pendingPeerId;
-	private byte[] verificationCodeSent;
+	private volatile byte[] verificationCodeSent;
-	private byte[] verificationCodeExpected;
+	private volatile byte[] verificationCodeExpected;
 
-	private PeerData peerData = null;
+	private volatile PeerData peerData = null;
 	private final ReentrantLock peerDataLock = new ReentrantLock();
 
 	/** Timestamp of when socket was accepted, or connected. */
-	private Long connectionTimestamp = null;
+	private volatile Long connectionTimestamp = null;
 
 	/** Peer's value of connectionTimestamp. */
-	private Long peersConnectionTimestamp = null;
+	private volatile Long peersConnectionTimestamp = null;
 
 	/** Version info as reported by peer. */
-	private VersionMessage versionMessage = null;
+	private volatile VersionMessage versionMessage = null;
 
 	/** Last PING message round-trip time (ms). */
-	private Long lastPing = null;
+	private volatile Long lastPing = null;
 	/** When last PING message was sent, or null if pings not started yet. */
-	private Long lastPingSent;
+	private volatile Long lastPingSent;
 
 	/** Latest block height as reported by peer. */
-	private Integer lastHeight;
+	private volatile Integer lastHeight;
 
 	/** Latest block signature as reported by peer. */
-	private byte[] lastBlockSignature;
+	private volatile byte[] lastBlockSignature;
 
 	/** Latest block timestamp as reported by peer. */
-	private Long lastBlockTimestamp;
+	private volatile Long lastBlockTimestamp;
 
 	/** Latest block generator public key as reported by peer. */
-	private byte[] lastBlockGenerator;
+	private volatile byte[] lastBlockGenerator;
 
 	// Constructors
 
@@ -151,6 +152,10 @@ public class Peer {
 		this.handshakeStatus = handshakeStatus;
 	}
 
+	public void resetHandshakeMessagePending() {
+		this.handshakeMessagePending = false;
+	}
+
 	public VersionMessage getVersionMessage() {
 		return this.versionMessage;
 	}
@@ -263,6 +268,11 @@ public class Peer {
 		return this.peerDataLock;
 	}
 
+	/* package */ void queueMessage(Message message) {
+		if (!this.pendingMessages.offer(message))
+			LOGGER.info(String.format("No room to queue message from peer %s - discarding", this));
+	}
+
 	@Override
 	public String toString() {
 		// Easier, and nicer output, than peer.getRemoteSocketAddress()
@@ -320,6 +330,7 @@ public class Peer {
 	 */
 	/* package */ void readChannel() throws IOException {
 		synchronized (this.byteBuffer) {
+			while(true) {
 				if (!this.socketChannel.isOpen() || this.socketChannel.socket().isClosed())
 					return;
 
@@ -329,10 +340,6 @@ public class Peer {
 					return;
 				}
 
-			if (bytesRead == 0)
-				// No room in buffer, or no more bytes to read
-				return;
-
 				LOGGER.trace(() -> String.format("Received %d bytes from peer %s", bytesRead, this));
 
 				while (true) {
@@ -347,9 +354,14 @@ public class Peer {
 						return;
 					}
 
-				if (message == null)
+					if (message == null && bytesRead == 0)
+						// No complete message in buffer and no more bytes to read from socket
 						return;
 
+					if (message == null)
+						// No complete message in buffer, but maybe more bytes to read from socket
+						break;
+
 					LOGGER.trace(() -> String.format("Received %s message with ID %d from peer %s", message.getType().name(), message.getId(), this));
 
 					BlockingQueue<Message> queue = this.replyQueues.get(message.getId());
@@ -367,16 +379,31 @@ public class Peer {
 						LOGGER.info(String.format("No room to queue message from peer %s - discarding", this));
 						return;
 					}
+
+					// Prematurely end any blocking channel select so that new messages can be processed
+					Network.getInstance().wakeupChannelSelector();
+				}
 			}
 		}
 	}
 
 	/* package */ ExecuteProduceConsume.Task getMessageTask() {
+		// If we are still handshaking and there is a message yet to be processed
+		// then don't produce another message task.
+		// This allows us to process handshake messages sequentially.
+		if (this.handshakeMessagePending)
+			return null;
+
 		final Message nextMessage = this.pendingMessages.poll();
 
 		if (nextMessage == null)
 			return null;
 
+		LOGGER.trace(() -> String.format("Produced %s message task from peer %s", nextMessage.getType().name(), this));
+
+		if (this.handshakeStatus != Handshake.COMPLETED)
+			this.handshakeMessagePending = true;
+
 		// Return a task to process message in queue
 		return () -> Network.getInstance().onMessage(this, nextMessage);
 	}