cleanup TCPRelay.cs

shadowsocks-csharp/Controller/Service/TCPRelay.cs

@@ -1,10 +1,12 @@
-using NLog;
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Net;
 using System.Net.Sockets;
 using System.Timers;
+
+using NLog;
+
 using Shadowsocks.Controller.Strategy;
 using Shadowsocks.Encryption;
 using Shadowsocks.Encryption.AEAD;
@@ -12,21 +14,22 @@ using Shadowsocks.Encryption.Exception;
 using Shadowsocks.Model;
 using Shadowsocks.Proxy;
 using Shadowsocks.Util.Sockets;
+
 using static Shadowsocks.Encryption.EncryptorBase;
 
 namespace Shadowsocks.Controller
 {
-    class TCPRelay : Listener.Service
+    internal class TCPRelay : Listener.Service
     {
         public event EventHandler<SSTCPConnectedEventArgs> OnConnected;
         public event EventHandler<SSTransmitEventArgs> OnInbound;
         public event EventHandler<SSTransmitEventArgs> OnOutbound;
         public event EventHandler<SSRelayEventArgs> OnFailed;
 
-        private static Logger logger = LogManager.GetCurrentClassLogger();
-        private ShadowsocksController _controller;
+        private static readonly Logger logger = LogManager.GetCurrentClassLogger();
+        private readonly ShadowsocksController _controller;
         private DateTime _lastSweepTime;
-        private Configuration _config;
+        private readonly Configuration _config;
 
         public ISet<TCPHandler> Handlers { get; set; }
 
@@ -42,7 +45,10 @@ namespace Shadowsocks.Controller
         {
             if (socket.ProtocolType != ProtocolType.Tcp
                 || (length < 2 || firstPacket[0] != 5))
+            {
                 return false;
+            }
+
             socket.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.NoDelay, true);
             TCPHandler handler = new TCPHandler(_controller, _config, socket);
 
@@ -52,7 +58,10 @@ namespace Shadowsocks.Controller
             handler.OnFailed += OnFailed;
             handler.OnClosed += (h, arg) =>
             {
-                lock (Handlers) { Handlers.Remove(handler); }
+                lock (Handlers)
+                {
+                    Handlers.Remove(handler);
+                }
             };
 
             IList<TCPHandler> handlersToClose = new List<TCPHandler>();
@@ -64,8 +73,12 @@ namespace Shadowsocks.Controller
                 {
                     _lastSweepTime = now;
                     foreach (TCPHandler handler1 in Handlers)
+                    {
                         if (now - handler1.lastActivity > TimeSpan.FromSeconds(900))
+                        {
                             handlersToClose.Add(handler1);
+                        }
+                    }
                 }
             }
             foreach (TCPHandler handler1 in handlersToClose)
@@ -133,7 +146,7 @@ namespace Shadowsocks.Controller
         public event EventHandler<SSRelayEventArgs> OnClosed;
         public event EventHandler<SSRelayEventArgs> OnFailed;
 
-        class AsyncSession
+        private class AsyncSession
         {
             public IProxy Remote { get; }
 
@@ -143,7 +156,7 @@ namespace Shadowsocks.Controller
             }
         }
 
-        class AsyncSession<T> : AsyncSession
+        private class AsyncSession<T> : AsyncSession
         {
             public T State { get; set; }
 
@@ -158,7 +171,7 @@ namespace Shadowsocks.Controller
             }
         }
 
-        private static Logger Logger = LogManager.GetCurrentClassLogger();
+        private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
 
         private readonly int _serverTimeout;
         private readonly int _proxyTimeout;
@@ -177,9 +190,9 @@ namespace Shadowsocks.Controller
 
         public DateTime lastActivity;
 
-        private ShadowsocksController _controller;
-        private ProxyConfig _config;
-        private Socket _connection;
+        private readonly ShadowsocksController _controller;
+        private readonly ProxyConfig _config;
+        private readonly Socket _connection;
 
         private IEncryptor _encryptor;
         private Server _server;
@@ -203,16 +216,16 @@ namespace Shadowsocks.Controller
         private int _totalWrite = 0;
 
         // remote -> local proxy (ciphertext, before decrypt)
-        private byte[] _remoteRecvBuffer = new byte[BufferSize];
+        private readonly byte[] _remoteRecvBuffer = new byte[BufferSize];
 
         // client -> local proxy (plaintext, before encrypt)
-        private byte[] _connetionRecvBuffer = new byte[BufferSize];
+        private readonly byte[] _connetionRecvBuffer = new byte[BufferSize];
 
         // local proxy -> remote (plaintext, after decrypt)
-        private byte[] _remoteSendBuffer = new byte[BufferSize];
+        private readonly byte[] _remoteSendBuffer = new byte[BufferSize];
 
         // local proxy -> client (ciphertext, before decrypt)
-        private byte[] _connetionSendBuffer = new byte[BufferSize];
+        private readonly byte[] _connetionSendBuffer = new byte[BufferSize];
 
         private bool _connectionShutdown = false;
         private bool _remoteShutdown = false;
@@ -230,6 +243,7 @@ namespace Shadowsocks.Controller
 
         private EndPoint _destEndPoint = null;
 
+        // TODO: decouple controller
         public TCPHandler(ShadowsocksController controller, Configuration config, Socket socket)
         {
             _controller = controller;
@@ -246,11 +260,13 @@ namespace Shadowsocks.Controller
             Server server = _controller.GetAServer(IStrategyCallerType.TCP, (IPEndPoint)_connection.RemoteEndPoint,
                 _destEndPoint);
             if (server == null || server.server == "")
+            {
                 throw new ArgumentException("No server configured");
+            }
 
             _encryptor = EncryptorFactory.GetEncryptor(server.method, server.password);
 
-            this._server = server;
+            _server = server;
 
             /* prepare address buffer length for AEAD */
             Logger.Trace($"_addrBufLength={_addrBufLength}");
@@ -267,14 +283,26 @@ namespace Shadowsocks.Controller
         private void CheckClose()
         {
             if (_connectionShutdown && _remoteShutdown)
+            {
                 Close();
+            }
+        }
+
+        private void ErrorClose(Exception e)
+        {
+            Logger.LogUsefulException(e);
+            Close();
         }
 
         public void Close()
         {
             lock (_closeConnLock)
             {
-                if (_closed) return;
+                if (_closed)
+                {
+                    return;
+                }
+
                 _closed = true;
             }
 
@@ -294,7 +322,7 @@ namespace Shadowsocks.Controller
             {
                 try
                 {
-                    var remote = _currentRemoteSession.Remote;
+                    IProxy remote = _currentRemoteSession.Remote;
                     remote.Shutdown(SocketShutdown.Both);
                     remote.Close();
                 }
@@ -315,7 +343,11 @@ namespace Shadowsocks.Controller
 
         private void HandshakeReceive()
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 int bytesRead = _firstPacketLength;
@@ -332,18 +364,23 @@ namespace Shadowsocks.Controller
                         HandshakeSendCallback, null);
                 }
                 else
+                {
                     Close();
+                }
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
         private void HandshakeSendCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 _connection.EndSend(ar);
@@ -355,20 +392,23 @@ namespace Shadowsocks.Controller
                 // +-----+-----+-------+------+----------+----------+
                 // Skip first 3 bytes, and read 2 more bytes to analysis the address.
                 // 2 more bytes is designed if address is domain then we don't need to read once more to get the addr length.
-                // TODO validate
+                // validate is unnecessary, we did it in first packet, but we can do it in future version
                 _connection.BeginReceive(_connetionRecvBuffer, 0, 3 + ADDR_ATYP_LEN + 1, SocketFlags.None,
-                    HandshakeReceive2Callback, null);
+                    AddressReceiveCallback, null);
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
-        private void HandshakeReceive2Callback(IAsyncResult ar)
+        private void AddressReceiveCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 int bytesRead = _connection.EndReceive(ar);
@@ -386,7 +426,7 @@ namespace Shadowsocks.Controller
                             // +----+-----+-------+------+----------+----------+
                             byte[] response = { 5, 0, 0, 1, 0, 0, 0, 0, 0, 0 };
                             _connection.BeginSend(response, 0, response.Length, SocketFlags.None,
-                                ResponseCallback, null);
+                                ConnectResponseCallback, null);
                             break;
                         case CMD_UDP_ASSOC:
                             ReadAddress(HandleUDPAssociate);
@@ -407,12 +447,11 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
-        private void ResponseCallback(IAsyncResult ar)
+        private void ConnectResponseCallback(IAsyncResult ar)
         {
             try
             {
@@ -422,8 +461,7 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -462,15 +500,19 @@ namespace Shadowsocks.Controller
 
         private void OnAddressFullyRead(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 int bytesRead = _connection.EndReceive(ar);
 
-                var states = (object[])ar.AsyncState;
+                object[] states = (object[])ar.AsyncState;
 
                 int bytesRemain = (int)states[0];
-                var onSuccess = (Action)states[1];
+                Action onSuccess = (Action)states[1];
 
                 if (bytesRead >= bytesRemain)
                 {
@@ -517,8 +559,7 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -546,7 +587,11 @@ namespace Shadowsocks.Controller
 
         private void ReadAll(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 if (ar.AsyncState != null)
@@ -564,13 +609,14 @@ namespace Shadowsocks.Controller
                             ReadAll, null);
                     }
                     else
+                    {
                         Close();
+                    }
                 }
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -635,7 +681,7 @@ namespace Shadowsocks.Controller
                     remote = new DirectConnect();
                 }
 
-                var session = new AsyncSession(remote);
+                AsyncSession session = new AsyncSession(remote);
                 lock (_closeConnLock)
                 {
                     if (_closed)
@@ -663,14 +709,13 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
         private void ProxyConnectTimer_Elapsed(object sender, ElapsedEventArgs e)
         {
-            var timer = (ProxyTimer)sender;
+            ProxyTimer timer = (ProxyTimer)sender;
             timer.Elapsed -= ProxyConnectTimer_Elapsed;
             timer.Enabled = false;
             timer.Dispose();
@@ -680,7 +725,7 @@ namespace Shadowsocks.Controller
             {
                 return;
             }
-            var proxy = timer.Session.Remote;
+            IProxy proxy = timer.Session.Remote;
 
             Logger.Info($"Proxy {proxy.ProxyEndPoint} timed out");
             proxy.Close();
@@ -695,15 +740,15 @@ namespace Shadowsocks.Controller
             }
             try
             {
-                var session = (AsyncSession<ProxyTimer>)ar.AsyncState;
+                AsyncSession<ProxyTimer> session = (AsyncSession<ProxyTimer>)ar.AsyncState;
                 ProxyTimer timer = session.State;
-                var destEndPoint = timer.DestEndPoint;
-                var server = timer.Server;
+                EndPoint destEndPoint = timer.DestEndPoint;
+                Server server = timer.Server;
                 timer.Elapsed -= ProxyConnectTimer_Elapsed;
                 timer.Enabled = false;
                 timer.Dispose();
 
-                var remote = session.Remote;
+                IProxy remote = session.Remote;
 
                 // Complete the connection.
                 remote.EndConnectProxy(ar);
@@ -739,14 +784,13 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
         private void DestConnectTimer_Elapsed(object sender, ElapsedEventArgs e)
         {
-            var timer = (ServerTimer)sender;
+            ServerTimer timer = (ServerTimer)sender;
             timer.Elapsed -= DestConnectTimer_Elapsed;
             timer.Enabled = false;
             timer.Dispose();
@@ -756,7 +800,7 @@ namespace Shadowsocks.Controller
                 return;
             }
 
-            var session = timer.Session;
+            AsyncSession session = timer.Session;
             Server server = timer.Server;
             OnFailed?.Invoke(this, new SSRelayEventArgs(_server));
             Logger.Info($"{server.FriendlyName()} timed out");
@@ -766,17 +810,21 @@ namespace Shadowsocks.Controller
 
         private void ConnectCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
-                var session = (AsyncSession<ServerTimer>)ar.AsyncState;
+                AsyncSession<ServerTimer> session = (AsyncSession<ServerTimer>)ar.AsyncState;
                 ServerTimer timer = session.State;
                 _server = timer.Server;
                 timer.Elapsed -= DestConnectTimer_Elapsed;
                 timer.Enabled = false;
                 timer.Dispose();
 
-                var remote = session.Remote;
+                IProxy remote = session.Remote;
                 // Complete the connection.
                 remote.EndConnectDest(ar);
 
@@ -784,7 +832,7 @@ namespace Shadowsocks.Controller
 
                 Logger.Debug($"Socket connected to ss server: {_server.FriendlyName()}");
 
-                var latency = DateTime.Now - _startConnectTime;
+                TimeSpan latency = DateTime.Now - _startConnectTime;
 
                 OnConnected?.Invoke(this, new SSTCPConnectedEventArgs(_server, latency));
 
@@ -799,8 +847,7 @@ namespace Shadowsocks.Controller
                 {
                     OnFailed?.Invoke(this, new SSRelayEventArgs(_server));
                 }
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -809,7 +856,7 @@ namespace Shadowsocks.Controller
             int available = Math.Min(_connection.Available, RecvSize - _firstPacketLength);
             if (available > 0)
             {
-                var size = _connection.Receive(_connetionRecvBuffer, _firstPacketLength, available,
+                int size = _connection.Receive(_connetionRecvBuffer, _firstPacketLength, available,
                     SocketFlags.None);
 
                 _firstPacketLength += size;
@@ -818,7 +865,11 @@ namespace Shadowsocks.Controller
 
         private void StartPipe(AsyncSession session)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 _startReceivingTime = DateTime.Now;
@@ -831,17 +882,20 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
         private void PipeRemoteReceiveCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
-                var session = (AsyncSession)ar.AsyncState;
+                AsyncSession session = (AsyncSession)ar.AsyncState;
                 int bytesRead = session.Remote.EndReceive(ar);
                 _totalRead += bytesRead;
 
@@ -884,20 +938,23 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
         private void PipeConnectionReceiveCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
                 int bytesRead = _connection.EndReceive(ar);
 
-                var session = (AsyncSession)ar.AsyncState;
-                var remote = session.Remote;
+                AsyncSession session = (AsyncSession)ar.AsyncState;
+                IProxy remote = session.Remote;
 
                 if (bytesRead > 0)
                 {
@@ -912,8 +969,7 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -943,12 +999,16 @@ namespace Shadowsocks.Controller
 
         private void PipeRemoteSendCallback(IAsyncResult ar)
         {
-            if (_closed) return;
+            if (_closed)
+            {
+                return;
+            }
+
             try
             {
-                var container = (object[])ar.AsyncState;
-                var session = (AsyncSession)container[0];
-                var bytesShouldSend = (int)container[1];
+                object[] container = (object[])ar.AsyncState;
+                AsyncSession session = (AsyncSession)container[0];
+                int bytesShouldSend = (int)container[1];
                 int bytesSent = session.Remote.EndSend(ar);
 
                 if (bytesSent > 0)
@@ -970,8 +1030,7 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
 
@@ -980,11 +1039,11 @@ namespace Shadowsocks.Controller
         {
             try
             {
-                var container = (object[])ar.AsyncState;
-                var session = (AsyncSession)container[0];
-                var bytesShouldSend = (int)container[1];
-                var bytesSent = _connection.EndSend(ar);
-                var bytesRemaining = bytesShouldSend - bytesSent;
+                object[] container = (object[])ar.AsyncState;
+                AsyncSession session = (AsyncSession)container[0];
+                int bytesShouldSend = (int)container[1];
+                int bytesSent = _connection.EndSend(ar);
+                int bytesRemaining = bytesShouldSend - bytesSent;
                 if (bytesRemaining > 0)
                 {
                     Logger.Info("reconstruct _remoteSendBuffer to re-send");
@@ -998,8 +1057,7 @@ namespace Shadowsocks.Controller
             }
             catch (Exception e)
             {
-                Logger.LogUsefulException(e);
-                Close();
+                ErrorClose(e);
             }
         }
     }