Refactor & cleanup logic around post-abort connection shutdown

I don't want to change this behaviour too much as it's all delicate and
poorly understood or tested, but this aborting state is what we hit
after most errors, and triggers a session shutdown, which then results
in downstream RST on any ongoing usage, much like any other silent
connection death.

This change tries to centralize that logic into the session (which is
slowly gaining ownership over session activity) and commonize logic
between the reader, writer & NIO service to link that all together.

Author: pimterry

app/src/main/java/tech/httptoolkit/android/vpn/Session.java

@@ -30,7 +30,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.DatagramChannel;
 import java.nio.channels.SelectionKey;
+import java.nio.channels.SocketChannel;
 import java.nio.channels.spi.AbstractSelectableChannel;
 
 /**
@@ -379,7 +381,28 @@ public void unsubscribeKey(int OP) {
 		}
 	}
 
-	public void closeSession() {
+	// Cleanly close a session - stopping all events, closing the upstream connection, and unregistering
+	// from the session manager
+	public void shutdown() {
+		AbstractSelectableChannel channel = this.getChannel();
+		this.cancelKey();
+
+		try {
+			if (channel instanceof SocketChannel) {
+				SocketChannel socketChannel = (SocketChannel) channel;
+				if (socketChannel.isConnected()) {
+					socketChannel.close();
+				}
+			} else {
+				DatagramChannel datagramChannel = (DatagramChannel) channel;
+				if (datagramChannel.isConnected()) {
+					datagramChannel.close();
+				}
+			}
+		} catch (IOException e) {
+			Log.e(TAG, e.toString());
+		}
+
 		this.sessionCloser.closeSession(this);
 	}
 

app/src/main/java/tech/httptoolkit/android/vpn/socket/SocketChannelReader.java

@@ -48,50 +48,26 @@ public SocketChannelReader(ClientPacketWriter writer) {
 	}
 
 	public void read(Session session) {
-		AbstractSelectableChannel channel = session.getChannel();
+		if (session.isAbortingConnection()) {
+			Log.d(TAG,"shutting down aborted connection on read -> "+ session);
+			session.shutdown();
+			return;
+		}
 
-		if(channel instanceof SocketChannel) {
+		AbstractSelectableChannel channel = session.getChannel();
+		if (channel instanceof SocketChannel) {
 			readTCP(session);
-		} else if(channel instanceof DatagramChannel){
+		} else if (channel instanceof DatagramChannel) {
 			readUDP(session);
 		} else {
 			return;
 		}
 
 		// Resubscribe to reads, so that we're triggered again if more data arrives later.
 		session.subscribeKey(SelectionKey.OP_READ);
-
-		if (session.isAbortingConnection()) {
-			Log.d(TAG,"removing aborted connection -> "+ session);
-			session.cancelKey();
-			if (channel instanceof SocketChannel){
-				try {
-					SocketChannel socketChannel = (SocketChannel) channel;
-					if (socketChannel.isConnected()) {
-						socketChannel.close();
-					}
-				} catch (IOException e) {
-					Log.e(TAG, e.toString());
-				}
-			} else {
-				try {
-					DatagramChannel datagramChannel = (DatagramChannel) channel;
-					if (datagramChannel.isConnected()) {
-						datagramChannel.close();
-					}
-				} catch (IOException e) {
-					e.printStackTrace();
-				}
-			}
-			session.closeSession();
-		}
 	}
 
 	private void readTCP(@NonNull Session session) {
-		if (session.isAbortingConnection()) {
-			return;
-		}
-
 		SocketChannel channel = (SocketChannel) session.getChannel();
 		int len;
 
@@ -211,11 +187,7 @@ private void readUDP(Session session){
 		int len;
 
 		try {
-			do{
-				if (session.isAbortingConnection()) {
-					break;
-				}
-
+			do {
 				len = channel.read(readBuffer);
 				if (len > 0) {
 					readBuffer.limit(len);
@@ -232,7 +204,7 @@ private void readUDP(Session session){
 							+ packetData.length);
 				}
 			} while(len > 0);
-		}catch(NotYetConnectedException ex){
+		} catch(NotYetConnectedException ex){
 			Log.e(TAG,"failed to read from unconnected UDP socket");
 		} catch (IOException e) {
 			Log.e(TAG,"Failed to read from UDP socket, aborting connection");

app/src/main/java/tech/httptoolkit/android/vpn/socket/SocketChannelWriter.java

@@ -48,6 +48,12 @@ public boolean isReadyToWrite(@NonNull Session session) {
 	}
 
 	public void write(@NonNull Session session) {
+		if (session.isAbortingConnection()) {
+			Log.d(TAG,"shutting down aborted connection on write -> "+ session);
+			session.shutdown();
+			return;
+		}
+
 		AbstractSelectableChannel channel = session.getChannel();
 		if (channel instanceof SocketChannel) {
 			writeTCP(session);
@@ -57,33 +63,6 @@ public void write(@NonNull Session session) {
 			// We only ever create TCP & UDP channels, so this should never happen
 			throw new IllegalArgumentException("Unexpected channel type: " + channel);
 		}
-
-		if (session.isAbortingConnection()) {
-			Log.d(TAG,"removing aborted connection -> " + session);
-			session.cancelKey();
-
-			if (channel instanceof SocketChannel) {
-				try {
-					SocketChannel socketChannel = (SocketChannel) channel;
-					if (socketChannel.isConnected()) {
-						socketChannel.close();
-					}
-				} catch (IOException e) {
-					e.printStackTrace();
-				}
-			} else if (channel instanceof DatagramChannel) {
-				try {
-					DatagramChannel datagramChannel = (DatagramChannel) channel;
-					if (datagramChannel.isConnected()) {
-						datagramChannel.close();
-					}
-				} catch (IOException e) {
-					e.printStackTrace();
-				}
-			}
-
-			session.closeSession();
-		}
 	}
 
 	private void writeUDP(Session session) {
@@ -102,8 +81,6 @@ private void writeUDP(Session session) {
 	}
 
 	private void writeTCP(Session session) {
-		if (!this.isReadyToWrite(session)) return;
-
 		try {
 			ProxyProtocolHandler setupHandler = session.getProxySetupHandler();