[Client] Fixed freezing of Subscription with sequential publishing

[AnatolyKochnev]

Bug

The client subscription fails to move messages from Subscription.m_incomingMessages to Subscription.m_messageCache, which leads to a complete freeze of the subscription publishing state.

Conditions Required to Reproduce

The issue occurs only when both of the following are met:

• Subscription.SequentialPublishing = true
• Subscription.SaveMessageInCache receives the first message with a SequenceNumber greater than 1 (availableSequenceNumbers is not relevant in this scenario)

Root Cause

When sequential publishing is enabled, the subscription cannot process incoming messages if there is a gap in sequence numbers.

Subscription.SaveMessageInCache already contains logic to handle gaps between messages. However, it does not handle the specific case where the first received message itself introduces the gap (i.e., when the sequence does not start from 1).

As a result:

• Sequential subscriptions become blocked due to the unresolved gap.
• For non-sequential subscriptions, republishing of initially missed messages may be skipped.

Proposed changes

• Added additional logic in Subscription.SaveMessageInCache to properly fill the gap between Subscription.m_lastSequenceNumberProcessed and IncomingMessage.SequenceNumber
• Added several unit tests in SubscriptionUnitTests to validate scenarios involving unordered messages passed to Subscription.SaveMessageInCache.

Notes

• The bug is difficult to reproduce in real-world environments but can be reliably reproduced in isolated unit tests.
• While writing the tests, an additional edge case was discovered: a message with an already processed SequenceNumber can be sent (see test UnorderedMessagesWouldBeLostForSequentialPublishingAsync).
  In this case, the subscription treats such a message as the latest received message.
  This appears to be another bug. For now, the corresponding test is marked as [Explicit] for further investigation.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply. You can also fill these out after creating the PR.

• Bugfix (non-breaking change which fixes an issue)
• Enhancement (non-breaking change which adds functionality)
• Test enhancement (non-breaking change to increase test coverage)
• Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
• Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING doc.
• I have signed the CLA.
• I ran tests locally with my changes, all passed.
• I fixed all failing tests in the CI pipelines.
• I fixed all introduced issues with CodeQL and LGTM.
• I have added tests that prove my fix is effective or that my feature works and increased code coverage.
• I have added necessary documentation (if appropriate).
• Any dependent changes have been merged and published in downstream modules.

[CLAassistant]

All committers have signed the CLA.

[marcschier]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[codecov]

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.98%. Comparing base (d25caff) to head (45fe9c6).
⚠️ Report is 129 commits behind head on master.

Files with missing lines	Patch %	Lines
...braries/Opc.Ua.Client/Subscription/Subscription.cs	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #3565       +/-   ##
===========================================
+ Coverage   51.86%   65.98%   +14.11%     
===========================================
  Files         370      459       +89     
  Lines       78618    97494    +18876     
  Branches    13650    16306     +2656     
===========================================
+ Hits        40779    64333    +23554     
+ Misses      33705    28067     -5638     
- Partials     4134     5094      +960     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[marcschier]

Any idea what the root cause of this is?

[AnatolyKochnev]

Before processing a message in a subscription, the following check must be performed:

(!State.SequentialPublishing || ValidSequentialPublishMessage(ii.Value))

and ValidSequentialPublishMessage:

{
    // If sequential publishing is enabled, only release messages in perfect sequence.
    return message.SequenceNumber <= m_lastSequenceNumberProcessed + 1 ||
        // reconnect / transfer subscription case
        m_resyncLastSequenceNumberProcessed ||
        // release the first message after wrapping around.
        (message.SequenceNumber == 1 && m_lastSequenceNumberProcessed == uint.MaxValue);
}

Here, a message with an outdated SequenceNumber can be accepted.
I tried to simply change it to: (message.SequenceNumber = m_lastSequenceNumberProcessed + 1),
It caused side effects and I abandoned the idea of ​​fixing it right now.

[marcschier]

Please remove regions.

[AnatolyKochnev]

Done

[marcschier]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).