Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 8, 2025

Proposed changes

Multiple threads calling CreateItemsAsync concurrently could create duplicate MonitoredItems on the server. The issue occurs because items are marked as created only after receiving the server response, allowing other threads to include the same items in separate create requests during the network round-trip.

Solution: Add internal Creating flag to track items with pending create requests.

Key Changes

  • MonitoredItemStatus: Add internal Creating property set before releasing lock, cleared after response
  • PrepareItemsToCreateAsync: Skip items where Created || Creating to prevent concurrent requests for same item
  • CreateItemsAsync: Clear Creating flag on exceptions to handle error paths
  • Test: Add ConcurrentCreateItemsNoDuplicates verifying 3 concurrent calls don't create duplicates

Example

Before fix - race condition:

Thread 1: Read items (Created=false) → Release lock → Send request → [Network delay]
Thread 2:                              Read items (Created=false) → Send request ❌ Duplicate!

After fix - prevented by Creating flag:

Thread 1: Read items (Created=false) → Set Creating=true → Release lock → Send request → Response sets Created=true
Thread 2:                                                   Read items (Creating=true) → Skip ✓

Related Issues

Fixes #2731

Types of changes

  • 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

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

Further comments

The fix is minimal and surgical - a single boolean flag prevents the race window. The Creating flag follows the same internal visibility pattern as other status tracking fields in MonitoredItemStatus. Exception handling ensures the flag is cleared even on network failures or validation errors, preventing items from becoming permanently stuck in the "creating" state.

Original prompt

This section details on the original issue you should resolve

<issue_title>MonitorItem is duplicated by Subscription.CreateItems</issue_title>
<issue_description>### Type of issue

  • Bug
  • Enhancement
  • Compliance
  • Question
  • Help wanted

Current Behavior

Context
Subscription holds a dictionary of all the monitored items. New items are added to the subscription via AddItems. Each item holds a created flag which is initially false. Once CreateItems is called, all the items having created = false are collected from the dictionary and transformed into CreateMonitoredItemsRequest which is sent to the server.

Possible bug
When multiple threads are used to create monitored items, each thread can freely call CreateItems.
Thread 1 accesses the dictionary, reading all the uncreated nodes from it. During this read, the dictionary is locked.
Once the read is over, the lock is dropped and requests are sent to the server. While thread 1 is awaiting a response to come back from the server, the monitored item's created flag stays in false state.

Before the response comes back, any other thread can read the dictionary and get the same, uncreated (yet requested) monitored item from the dictionary and include it to the new request. Now the same monitored thread is included in two separate requests and being created twice. This scenario is illustrated in the picture below. Now we have two monitored items in the server and notifications are sent twice to the same monitored node on the client-side.

Green color: thread 1
Orange color: thread 2
subscription-createitems-multiple-threads

Question
I guess, it's legit to be able to have create multiple monitored items for the same node when explicitly done so. To me, the current behavior feels very subtle.
Is this behavior known and intended? Basically, a developer must know that calling CreateItems is not thread-safe while calling AddItems is.

Expected Behavior

No response

Steps To Reproduce

No response

Environment

Opc.Ua.Client 1.4.367
Opc.Ua.Core 1.4.367

Anything else?

No response</issue_description>

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

@mregen @laaksonel thanks for the report. Apparently the behavior you describe could cause issues when race conditions occur. Tagged as bug.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI and others added 2 commits December 8, 2025 23:13
Add Creating flag to prevent duplicate items when CreateItemsAsync is called concurrently.
- Add internal Creating flag to MonitoredItemStatus
- Check Creating flag in PrepareItemsToCreateAsync
- Set Creating flag before releasing lock
- Clear Creating flag in SetCreateResult (success or failure)
- Add exception handling in CreateItemsAsync to clear Creating flag on errors
- Add test for concurrent CreateItemsAsync calls

Co-authored-by: romanett <[email protected]>
Copilot AI changed the title [WIP] Fix duplication of MonitorItem by Subscription.CreateItems Fix race condition causing duplicate MonitoredItems in concurrent CreateItemsAsync calls Dec 8, 2025
Copilot AI requested a review from romanett December 8, 2025 23:34
@romanett romanett marked this pull request as ready for review December 9, 2025 05:46
@marcschier marcschier merged commit bf67f39 into master Dec 9, 2025
129 of 130 checks passed
@romanett romanett deleted the copilot/fix-duplicate-monitor-items branch December 9, 2025 19:02
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.

MonitorItem is duplicated by Subscription.CreateItems

4 participants