issue #309: obey a GapFill even if it replaces...

... a message that was processed off the queue

This is relevant only during a very specific
and uncommon Gap Fill situation at the
end of a ResendRequest series

Author: gbirchmeier

AcceptanceTest/definitions/server/fix44/issue309_GapFillSkip.def

@@ -7,7 +7,7 @@ I8=FIX.4.4
35=A
34=1
49=TW
52=<TIME>
56=ISLD
98=0
108=2

 E8=FIX.4.4
35=A
34=1
49=ISLD
52=00000000-00:00:00.000
56=TW
98=0
108=2
10=0

 
 # News msg to bump the seq up
-I8=FIX.4.4
35=B
34=2
49=TW
52=<TIME>
56=ISLD
148=no_echo
33=0

+I8=FIX.4.4
35=B
34=2
49=TW
52=<TIME>
56=ISLD
148=no_echo

 
 # Heartbeat with seq 5 (when 3 is expected)
 I8=FIX.4.4
35=0
34=5
49=TW
52=<TIME>
56=ISLD

@@ -16,8 +16,8 @@ I8=FIX.4.4
35=0
34=5
49=TW
52=<TIME>
56=ISLD

 E8=FIX.4.4
35=2
34=2
49=ISLD
52=00000000-00:00:00.000
56=TW
7=3
16=0
10=0

 
 # Resend 3 and 4
-I8=FIX.4.4
35=B
34=3
43=Y
49=TW
52=<TIME>
56=ISLD
148=no_echo
33=0

-I8=FIX.4.4
35=B
34=4
43=Y
49=TW
52=<TIME>
56=ISLD
148=no_echo
33=0

+I8=FIX.4.4
35=B
34=3
43=Y
49=TW
52=<TIME>
56=ISLD
148=no_echo

+I8=FIX.4.4
35=B
34=4
43=Y
49=TW
52=<TIME>
56=ISLD
148=no_echo

 sleep(0.2)
 
 # Session now processes the Heartbeat (seq=5) from its queue
@@ -28,6 +28,6 @@ E8=FIX.4.4
35=0
34=3
49=ISLD
52=00000000-00:00:00.000
56=TW
10=0

 # The EndSeqNo is 7, completely skipping over 6.
 I8=FIX.4.4
35=4
34=5
49=TW
52=<TIME>
56=ISLD
43=Y
122=<TIME-1>
36=7
123=Y

 
-# Ensure that client obeyed the SequenceReset and set NextExpected=7
-I8=FIX.4.4
35=B
34=7
49=TW
52=<TIME>
56=ISLD
148=foo
33=0

-E8=FIX.4.4
35=B
34=4
49=ISLD
52=00000000-00:00:00.000
56=TW
33=0
148=foo

+# Do an echo to ensure that client obeyed the SequenceReset and set NextExpected=7
+I8=FIX.4.4
35=B
34=7
49=TW
52=<TIME>
56=ISLD
148=echo: NextExpected/7

+E8=FIX.4.4
35=B
34=4
49=ISLD
52=00000000-00:00:00.000
56=TW
148=NextExpected/7
10=0

QuickFIXn/Session.cs

@@ -523,6 +523,7 @@ public void Next()
         public void Next(string msgStr)
         {
             NextMessage(msgStr);
+            _state.LastProcessedMessageWasQueued = false;
             NextQueued();
         }
 
@@ -962,8 +963,20 @@ public bool Verify(Message msg, bool checkTooHigh = true, bool checkTooLow = tru
                     }
                     if (checkTooLow && IsTargetTooLow(msgSeqNum))
                     {
-                        DoTargetTooLow(msg, msgSeqNum);
-                        return false;
+                        if (_state.LastProcessedMessageWasQueued
+                            && msg.Header.GetString(35) == MsgType.SEQUENCE_RESET
+                            && msg.GetBoolean(Tags.GapFillFlag))
+                        {
+                            Log.Log(LogLevel.Warning,
+                                "SequenceReset-GapFill 34={MsgSeqNum} is too low (expected {NextTargetMsgSeqNum}), but in this case I'm going to obey it anyway",
+                                msgSeqNum, _state.NextTargetMsgSeqNum);
+                            // This is an uncommon situation, see #309
+                        }
+                        else
+                        {
+                            DoTargetTooLow(msg, msgSeqNum);
+                            return false;
+                        }
                     }
 
                     if (IsResendRequested)
@@ -1594,6 +1607,7 @@ protected bool NextQueued(SeqNumType num)
                 {
                     NextMessage(msg.ConstructString());
                 }
+                _state.LastProcessedMessageWasQueued = true;
                 return true;
             }
             return false;

QuickFIXn/SessionState.cs

@@ -48,6 +48,12 @@ public bool IsInitiator
 
         public ILogger Log { get; }
 
+        /// <summary>
+        /// True if the last message processed was an admin message from the queue.
+        /// Needed for an obscure SequenceReset scenario (see issue #390).
+        /// </summary>
+        internal bool LastProcessedMessageWasQueued { get; set; }
+
         #endregion
 
         #region Synchronized Properties

RELEASE_NOTES.md

@@ -45,7 +45,7 @@ What's New
 * #939 - minor checkTooHigh/checkTooLow refactor in Session.cs (gbirchmeier)
 * #941 - clarify ResendRequest-related log message, add UT coverage for Session (gbirchmeier)
 * #895 - fix: When SSLCACertificate is empty an error is logged and it fails to start (dckorben)
-* #942 - fix #942: field 369 (LastMsgSeqNumProcessed) wrong in ResendRequest message (gbirchmeier)
+* #942 - fix: field 369 (LastMsgSeqNumProcessed) wrong in ResendRequest message (gbirchmeier)
 * #940 - Create an alternate CharEncoding.GetBytes impl which uses ArrayPool to improve memory performance (VAllens)
 * #951 - fix: restore Session disconnect during SocketInitiatorThread.Read exception (gbirchmeier/trevor-bush)
 * #963 - fix: concurrency bug with NonSessionLog on Windows (gbirchmeier)
@@ -57,6 +57,8 @@ What's New
 * #969 - correct LinesOfText in DDs to not be required; make ATs not auto-echo News (gbirchmeier)
 * #965 - Reusing StringBuilder with Object Pooling (VAllens)
 * #980 - fix: ToJSON returns invalid json when content contains newlines/tabs/etc (Rob-Hague)
+* #309 - fix: obey a SeqReset-GapFill even if it 'replaces' a message that was processed off
+         queue in a ResendRequest series (gbirchmeier/oclancy)
 
 ### v1.13.1
 * backport #951 to 1.13

UnitTests/SessionTest.cs

@@ -821,8 +821,11 @@ public void TestSequenceResetDuringResendRequestIsProcessed()
     }
 
     [Test]
-    public void TestGapFillShouldNotBeIgnoredIfPossDup()
+    public void TestObeyGapFillIfItReplacesAMessageOffTheQueue()
     {
+        // issue #309: If GapFill has the same seqno of a message that was
+        //   processed off the queue, obey it anyway
+
         Assert.That(_session!.IgnorePossDupResendRequests, Is.EqualTo(false));
         _session.RequiresOrigSendingTime = false; // default is true
         Logon();
@@ -838,6 +841,7 @@ public void TestGapFillShouldNotBeIgnoredIfPossDup()
         Assert.That(resendRequest.EndSeqNo.Value, Is.EqualTo(0));
 
         Assert.That(_session.NextTargetMsgSeqNum, Is.EqualTo(3));
+        Assert.That(_session.IsResendRequested);
 
         // Resends
         SendNOSMessage(3, possDupFlag: true);