Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 10, 2025

Problem

The OPC UA client's Close method does not properly terminate outstanding publish requests before closing the session, which violates the OPC UA standard and can lead to ServiceFault responses from the server.

According to OPC UA Part 4, Section 5.6.4: "Clients are urged to wait for all outstanding requests to complete before submitting the CloseSession request."

Current Behavior

When closing a session with active subscriptions:

  1. The session is closed immediately
  2. Outstanding publish requests are left pending
  3. The server may return ServiceFault responses for these orphaned requests

Expected Behavior

Before closing a session, the client should:

  1. Stop sending new publish requests
  2. Wait for outstanding publish requests to complete
  3. Cancel any remaining requests that don't complete in time
  4. Then send the CloseSession request

Solution

This PR adds proper handling of outstanding publish requests during session close:

1. New Configuration Property

Added PublishRequestCancelWaitTimeout property to control the wait behavior:

// Default: Wait up to 5 seconds before canceling
await session.CloseAsync();

// Cancel immediately without waiting
session.PublishRequestCancelWaitTimeout = 0;
await session.CloseAsync();

// Wait indefinitely for all requests to complete
session.PublishRequestCancelWaitTimeout = -1;
await session.CloseAsync();

// Custom timeout
session.PublishRequestCancelWaitTimeout = 10000; // 10 seconds
await session.CloseAsync();

2. Enhanced Close Flow

The CloseAsync method now:

  1. Stops the keep-alive timer (prevents new publish requests)
  2. Identifies outstanding publish requests
  3. Waits for them to complete (up to configured timeout)
  4. Cancels remaining requests using the OPC UA Cancel service
  5. Proceeds with the CloseSession request

3. Implementation Details

  • Helper Method: WaitForOrCancelOutstandingPublishRequestsAsync() handles the wait/cancel logic
  • Cancellation Support: Properly handles CancellationToken for early termination
  • Logging: Logs information about outstanding requests and cancellation actions
  • Error Handling: Continues with session close even if cancel operations fail

Testing

  • Added unit tests to verify outstanding requests are properly cancelled
  • Added tests to verify property configuration works correctly
  • All existing tests pass with no regressions
  • Build completes with 0 warnings and 0 errors

Breaking Changes

None. This change is backward compatible with a sensible default timeout of 5 seconds.

Benefits

Standards Compliant: Follows OPC UA Part 4 recommendations
Reduces ServiceFaults: Prevents errors from orphaned publish requests
Configurable: Applications can choose the appropriate wait strategy
Backward Compatible: Default behavior is a reasonable compromise
Reliable: Improves client stability during shutdown

Fixes #1821

Original prompt

This section details on the original issue you should resolve

<issue_title>The client close method does not properly terminate the outstanding requests</issue_title>
<issue_description>### Type of issue

  • Bug
  • Enhancement
  • Compliance
  • Question
  • Help wanted

Current Behavior

Close method closes sessions without stopping/waiting outstanding request to properly terminated.

Expected Behavior

According to the OPCUA standard: Clients are urged to wait for all outstanding requests to complete before submitting the CloseSession request. (extracted from 5.6.4 CloseSesison OPC Unified Architecture, Part 4)

The client does not call any cancel service before calling the closeSession request. In consequence, the client can receive ServiceFault result for publish request that have not been properly terminated (by waiting the response or calling the cancel service on it)
image

I think that the the Close(int timeout) method should stop the monitorItems publishing and call the cancel service on outstanding requests before sending the closeSession request.

Regards

Steps To Reproduce

  1. Compile Sample client
  2. Connect to a server in none security
  3. Create a session, a subscription, a monitorItems
  4. Launch wireshark
  5. Close the session
    6.The server mays has sent Fault answer to the last publish service request sent by the client.

Environment

- OS: Windows 10
- Environment:Visual studio 2019 16.11.3
- Runtime: NT 4.6.2
- Nuget Version:1.4.368.33
- Component:Opc.Ua.Client
- Server:ReferenceServer
- Client: ReferenceClient

Anything else?

Opc.Ua.Client\Session.cs\public virtual StatusCode Close(int timeout)</issue_description>

<agent_instructions>Please update the Client Library to cancel outstanding publish requests on Session Close. Please add a Property that defines a wait timeout before publish requests are cancelled.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@mregen @GregoireG-C , since subscription can be transferred, the Close method does not try to close subscriptions or cancel publish request by default. @romanett I do not fully agree to the explanation. Outstanding publish requests are not transferred to the new session. So on session close they shall all be cancelled or waited for completion by the client. Also the client shall stop publishing when close is called.

Fixes #1778


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ romanett
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@Copilot Copilot AI changed the title [WIP] Fix client close method to properly terminate requests Fix client Close method to properly terminate outstanding publish requests Oct 10, 2025
Copilot finished work on behalf of romanett October 10, 2025 15:51
@Copilot Copilot AI requested a review from romanett October 10, 2025 15:51
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 87.96296% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.76%. Comparing base (2bee7e1) to head (69b5600).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
Libraries/Opc.Ua.Client/Session/Session.cs 87.73% 10 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3254      +/-   ##
==========================================
+ Coverage   57.74%   57.76%   +0.02%     
==========================================
  Files         365      365              
  Lines       79419    79495      +76     
  Branches    13865    13880      +15     
==========================================
+ Hits        45862    45922      +60     
- Misses      29310    29323      +13     
- Partials     4247     4250       +3     

☔ 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.

@romanett romanett marked this pull request as ready for review October 14, 2025 05:41
e.AcknowledgementsToSend.Clear();
}

/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests are really bad, either improve or remove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate references after export via NodeStateCollection.SaveAsNodeSet2 The client close method does not properly terminate the outstanding requests

3 participants