Improve locking of the AniDB Socket Handler

ShokoAnime · Nov 10, 2024 · 8e41e48 · 8e41e48
1 parent d3fecd9
commit 8e41e48
Show file tree

Hide file tree

Showing 2 changed files with 170 additions and 121 deletions.
diff --git a/Shoko.Server/Providers/AniDB/UDP/AniDBSocketHandler.cs b/Shoko.Server/Providers/AniDB/UDP/AniDBSocketHandler.cs
@@ -110,67 +110,71 @@ private void EmptyBuffer()
     public async Task<bool> TryConnection()
     {
         if (IsConnected) return true;
-        await _semaphore.WaitAsync();
-        // Don't send Expect 100 requests. These requests aren't always supported by remote internet devices, in which case can cause failure.
-        ServicePointManager.Expect100Continue = false;
-
         try
         {
-            _localIpEndPoint = new IPEndPoint(IPAddress.Any, _clientPort);
-
-            // we use bind() here (normally only for servers, not clients) instead of connect() because of this:
-            /*
-             * Local Port
-             *  A client should select a fixed local port >1024 at install time and reuse it for local UDP Sockets. If the API sees too many different UDP Ports from one IP within ~1 hour it will ban the IP. (So make sure you're reusing your UDP ports also for testing/debugging!)
-             *  The local port may be hardcoded, however, an option to manually specify another port should be offered.
-             */
-            _aniDBSocket.Bind(_localIpEndPoint);
-            _aniDBSocket.ReceiveTimeout = ReceiveTimeoutMs;
-
-            _logger.LogInformation("Bound to local address: {Local} - Port: {ClientPort} ({Family})", _localIpEndPoint,
-                _clientPort, _localIpEndPoint.AddressFamily);
-        }
-        catch (Exception ex)
-        {
-            _logger.LogError(ex, "Could not bind to local port");
-            _semaphore.Release();
-            IsConnected = false;
-            return false;
-        }
+            await _semaphore.WaitAsync();
+            // Don't send Expect 100 requests. These requests aren't always supported by remote internet devices, in which case can cause failure.
+            ServicePointManager.Expect100Continue = false;
 
-        try
-        {
-            var remoteHostEntry = await Dns.GetHostEntryAsync(_serverHost).WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false);
-            _remoteIpEndPoint = new IPEndPoint(remoteHostEntry.AddressList[0], _serverPort);
+            try
+            {
+                _localIpEndPoint = new IPEndPoint(IPAddress.Any, _clientPort);
+
+                // we use bind() here (normally only for servers, not clients) instead of connect() because of this:
+                /*
+                 * Local Port
+                 *  A client should select a fixed local port >1024 at install time and reuse it for local UDP Sockets. If the API sees too many different UDP Ports from one IP within ~1 hour it will ban the IP. (So make sure you're reusing your UDP ports also for testing/debugging!)
+                 *  The local port may be hardcoded, however, an option to manually specify another port should be offered.
+                 */
+                _aniDBSocket.Bind(_localIpEndPoint);
+                _aniDBSocket.ReceiveTimeout = ReceiveTimeoutMs;
+
+                _logger.LogInformation("Bound to local address: {Local} - Port: {ClientPort} ({Family})", _localIpEndPoint,
+                    _clientPort, _localIpEndPoint.AddressFamily);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Could not bind to local port");
+                IsConnected = false;
+                return false;
+            }
+
+            try
+            {
+                var remoteHostEntry = await Dns.GetHostEntryAsync(_serverHost).WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false);
+                _remoteIpEndPoint = new IPEndPoint(remoteHostEntry.AddressList[0], _serverPort);
 
-            _logger.LogInformation("Bound to remote address: {Address} : {Port}", _remoteIpEndPoint.Address,
-                _remoteIpEndPoint.Port);
+                _logger.LogInformation("Bound to remote address: {Address} : {Port}", _remoteIpEndPoint.Address,
+                    _remoteIpEndPoint.Port);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Could not bind to remote port");
+                IsConnected = false;
+                return false;
+            }
+
+            IsConnected = true;
+            return true;
         }
-        catch (Exception ex)
+        finally
         {
-            _logger.LogError(ex, "Could not bind to remote port");
             _semaphore.Release();
-            IsConnected = false;
-            return false;
         }
-
-        _semaphore.Release();
-        IsConnected = true;
-        return true;
     }
 
     public async ValueTask DisposeAsync()
     {
         await _semaphore.WaitAsync();
         GC.SuppressFinalize(this);
-        if (_aniDBSocket == null)
-        {
-            IsConnected = false;
-            return;
-        }
-
         try
         {
+            if (_aniDBSocket == null)
+            {
+                IsConnected = false;
+                return;
+            }
+
             if (_aniDBSocket.Connected)
             {
                 _aniDBSocket.Shutdown(SocketShutdown.Both);
@@ -181,15 +185,16 @@ public async ValueTask DisposeAsync()
             {
                 await _aniDBSocket.DisconnectAsync(false);
             }
+
+            _aniDBSocket.Close();
+            _logger.LogInformation("Closed AniDB Connection");
         }
         catch (SocketException ex)
         {
             _logger.LogError(ex, "Failed to Shutdown and Disconnect the connection to AniDB");
         }
         finally
         {
-            _aniDBSocket.Close();
-            _logger.LogInformation("Closed AniDB Connection");
             _semaphore.Release();
             _semaphore.Dispose();
             IsConnected = false;
@@ -200,14 +205,14 @@ public void Dispose()
     {
         _semaphore.Wait();
         GC.SuppressFinalize(this);
-        if (_aniDBSocket == null)
-        {
-            IsConnected = false;
-            return;
-        }
-
         try
         {
+            if (_aniDBSocket == null)
+            {
+                IsConnected = false;
+                return;
+            }
+
             if (_aniDBSocket.Connected)
             {
                 _aniDBSocket.Shutdown(SocketShutdown.Both);
@@ -218,15 +223,16 @@ public void Dispose()
             {
                 _aniDBSocket.Disconnect(false);
             }
+
+            _aniDBSocket.Close();
+            _logger.LogInformation("Closed AniDB Connection");
         }
         catch (SocketException ex)
         {
             _logger.LogError(ex, "Failed to Shutdown and Disconnect the connection to AniDB");
         }
         finally
         {
-            _aniDBSocket.Close();
-            _logger.LogInformation("Closed AniDB Connection");
             _semaphore.Release();
             _semaphore.Dispose();
             IsConnected = false;