🐛 Fix engine crash due to a negative calculated time to move (#555)

Fix `Math.Clamp` usage when millisecondsLeft < 50, which caused engine crashes
lynx-chess · Dec 31, 2023 · 1b0da54 · 1b0da54
1 parent 2fee1b2
commit 1b0da54
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 20 deletions.
diff --git a/src/Lynx/Engine.cs b/src/Lynx/Engine.cs
@@ -99,15 +99,15 @@ public async Task<SearchResult> BestMove(GoCommand goCommand)
             {
                 // Inspired by Alexandria: time overhead to avoid timing out in the engine-gui communication process
                 millisecondsLeft -= 50;
-                Math.Clamp(millisecondsLeft, 50, int.MaxValue); // Avoiding 0/negative values
+                millisecondsLeft = Math.Clamp(millisecondsLeft, 50, int.MaxValue); // Avoiding 0/negative values
 
                 // 1/30, suggested by Serdra (EP discord)
                 decisionTime = Convert.ToInt32(Math.Min(0.5 * millisecondsLeft, (millisecondsLeft * 0.03333) + millisecondsIncrement));
             }
             else
             {
                 millisecondsLeft -= 500;
-                Math.Clamp(millisecondsLeft, 50, int.MaxValue); // Avoiding 0/negative values
+                millisecondsLeft = Math.Clamp(millisecondsLeft, 50, int.MaxValue); // Avoiding 0/negative values
 
                 decisionTime = Convert.ToInt32((millisecondsLeft / goCommand.MovesToGo) + millisecondsIncrement);
             }

diff --git a/src/Lynx/Search/Helpers.cs b/src/Lynx/Search/Helpers.cs
@@ -154,27 +154,36 @@ private static int ScoreHistoryMove(int score, int rawHistoryBonus)
         return score + rawHistoryBonus - (score * Math.Abs(rawHistoryBonus) / Configuration.EngineSettings.History_MaxMoveValue);
     }
 
+#pragma warning disable RCS1226 // Add paragraph to documentation comment
+#pragma warning disable RCS1243 // Duplicate word in a comment
+    /// <summary>
+    /// When asked to copy an incomplete PV one level ahead, clears the rest of the PV Table+
+    /// PV Table at depth 3
+    /// Copying 60 moves
+    /// src: 250, tgt: 190
+    ///  0   Qxb2   Qxb2   h4     a8     a8     a8     a8     a8
+    ///  64         b1=Q   exf6   Kxf6   a8     a8     a8     a8
+    ///  127               a8     b1=Q   Qxb1   Qxb1   a8     a8
+    ///  189                      Qxb1   Qxb1   Qxb1   a8     a8
+    ///  250                             a8     Qxb1   a8     a8
+    ///  310                                    a8     a8     a8
+    ///
+    /// PV Table at depth 3
+    ///  0   Qxb2   Qxb2   h4     a8     a8     a8     a8     a8
+    ///  64         b1=Q   exf6   Kxf6   a8     a8     a8     a8
+    ///  127               a8     b1=Q   Qxb1   Qxb1   a8     a8
+    ///  189                      Qxb1   a8     a8     a8     a8
+    ///  250                             a8     a8     a8     a8
+    ///  310                                    a8     a8     a8
+    /// </summary>
+    /// <param name="target"></param>
+    /// <param name="source"></param>
+    /// <param name="moveCountToCopy"></param>
+#pragma warning restore RCS1243 // Duplicate word in a comment
+#pragma warning restore RCS1226 // Add paragraph to documentation comment
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private void CopyPVTableMoves(int target, int source, int moveCountToCopy)
     {
-        // When asked to copy an incomplete PV one level ahead, clears the rest of the PV Table+
-        // PV Table at depth 3
-        // Copying 60 moves
-        // src: 250, tgt: 190
-        //  0   Qxb2   Qxb2   h4     a8     a8     a8     a8     a8
-        //  64         b1=Q   exf6   Kxf6   a8     a8     a8     a8
-        //  127               a8     b1=Q   Qxb1   Qxb1   a8     a8
-        //  189                      Qxb1   Qxb1   Qxb1   a8     a8
-        //  250                             a8     Qxb1   a8     a8
-        //  310                                    a8     a8     a8
-        //
-        // PV Table at depth 3
-        //  0   Qxb2   Qxb2   h4     a8     a8     a8     a8     a8
-        //  64         b1=Q   exf6   Kxf6   a8     a8     a8     a8
-        //  127               a8     b1=Q   Qxb1   Qxb1   a8     a8
-        //  189                      Qxb1   a8     a8     a8     a8
-        //  250                             a8     a8     a8     a8
-        //  310                                    a8     a8     a8
         if (_pVTable[source] == default)
         {
             Array.Clear(_pVTable, target, _pVTable.Length - target);