Commit Graph

4 Commits

Author SHA1 Message Date
romanett 1af191ec26 Drop the .Common suffix from the three GDS projects (#4438)
# Description

Every other companion specification in the stack ships as `<Family>`,
`<Family>.Client` and `<Family>.Server` — `Opc.Ua.Di`, `Opc.Ua.ISA95`,
`Opc.Ua.Positioning`, `Opc.Ua.Robotics`, `Opc.Ua.Vision`,
`Opc.Ua.WotCon`, `Opc.Ua.XRegistry`, and `Opc.Ua.Lds.Server` without a
family package. GDS was the one exception, carrying a `.Common` suffix
that named nothing.

| Before | After |
| --- | --- |
| `src/Opc.Ua.Gds.Common` | `src/Opc.Ua.Gds` |
| `src/Opc.Ua.Gds.Client.Common` | `src/Opc.Ua.Gds.Client` |
| `src/Opc.Ua.Gds.Server.Common` | `src/Opc.Ua.Gds.Server` |

## No source changes were needed

`RootNamespace` was already the final name in all three projects — the
namespaces have always been `Opc.Ua.Gds`, `Opc.Ua.Gds.Client` and
`Opc.Ua.Gds.Server`, and only `AssemblyName` and `PackageId` disagreed.
Nothing that compiles against these assemblies changes a `using`
directive; the diff outside the moves is entirely ids, paths and prose.

Directories were moved with `git mv`, and every one of the 73 file moves
is detected as a rename at 100% similarity (bar the handful that also
had a name in their text), so `git log --follow` and blame survive.

## Consumer impact

The package ids change, which is visible to anyone consuming the NuGet
packages:

| 1.5.378 package | 2.0 package |
| --- | --- |
| `OPCFoundation.NetStandard.Opc.Ua.Gds.Common` |
`OPCFoundation.NetStandard.Opc.Ua.Gds` |
| `OPCFoundation.NetStandard.Opc.Ua.Gds.Client.Common` |
`OPCFoundation.NetStandard.Opc.Ua.Gds.Client` |
| `OPCFoundation.NetStandard.Opc.Ua.Gds.Server.Common` |
`OPCFoundation.NetStandard.Opc.Ua.Gds.Server` |

`docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it
is the document that undertakes to cover NuGet renames, and had no GDS
entry — and the migration skill's package table gains the two rows a
1.5.378 consumer needs. That is the only hand-written content in this
PR; everything else is mechanical.

Assembly names follow the package ids, so the section also notes that
binding redirects, trimming descriptors and signing manifests naming the
files directly have to be updated.

## Build infrastructure

Everything naming the projects or their output moves with them:

- `UA.slnx` and `tools/MigrationAnalyzer.slnx` (both re-sorted — the
project lists are alphabetical, and `Gds` now sorts before
`Gds.Client`).
- `.azurepipelines/expected-packages.txt` — 3 ids, also re-sorted for
the same reason.
- `.azurepipelines/signlistDebug.txt` and `signlistRelease.txt` — 18
paths each. Their internal ordering is left alone: that group was
already unsorted, and reordering a signing manifest is diff noise.
- `samples/Reference/ConsoleReferenceServer/Dockerfile`, the two
metapackage nuspecs, the analyzer nuspec's package dependencies, and 8
`.csproj` references across tests, samples and tools.

`preview-pack.slnx` is generated from `UA.slnx` by `generate-slnx.ps1`,
so there is nothing static to edit. Nothing under `.github/workflows`,
and no `.ps1`/`.yml` in `.azurepipelines`, names these projects.

The migration analyzer's shim tree mirrors the source project layout by
its own documented convention, so `Gds.Client.Common/` becomes
`Gds.Client/` and its readme table follows.

## Deliberately not changed

Prose naming past work: `plans/23` still records that its deferral
happened during the "GDS Client.Common modernization", which is the name
that effort had. File *paths* in the same document were updated.

## Validation

`net10.0`:

- `UA.slnx`: 0 errors. 14 warnings, all pre-existing
CA1861/CA1850/CA1307 in `XRegistry`, `Types.Tests` and `WotCon.Tests` —
none in anything this PR touches.
- Assemblies build as `Opc.Ua.Gds.dll`, `Opc.Ua.Gds.Client.dll`,
`Opc.Ua.Gds.Server.dll`, with no `*.Common.dll` produced anywhere.
- `Opc.Ua.Gds.Tests`: 1,116 passed, 0 failed, 54 skipped.
- `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped.
- A repo-wide grep for the old names returns only the intentional old →
new mappings in the migration docs.

## Checklist

- [x] Existing tests pass
- [x] No new tests — this is a rename with no behaviour change
- [x] Consumer-visible package rename documented in the migration guide

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-07 20:08:54 +02:00
romanett 414957c88a Reduce ISubscription: internalize the publish pipeline, delete the dead members (#4349)
# Description

Reduces the server subscription API surface per the measurements from
issue #4185.

## ISubscription: 42 → 30 members (#4185)

A call-site census of the 11 "publish-pipeline protocol" members plus
`Publish` (all 68 `src/` projects, samples, tools, fuzzing, tests) found
**zero callers outside `Opc.Ua.Server`** and split the members three
ways:

- **Deleted (4):** `ItemReadyToPublish` and `ItemNotificationsAvailable`
were dead — zero callers repo-wide and implementation bodies commented
out since before 1.5.378 — the obsolete parameterless `SessionClosed()`
had zero callers, and `TransferSessionAsync` (the one-shot direct
transfer, which bypassed the reservation protecting a transfer against a
concurrently closing source session) lost its last caller once the
manager always uses the claim/prepare/commit protocol.
- **Internalized (8):** `PublishTimerExpired`, `Acknowledge`,
`PublishTimeout`, `SubscriptionTransferred`,
`AvailableSequenceNumbersForRetransmission`, `QueueOverflowHandler`,
`SessionClosed(ISession)` and `Publish` move to `internal interface
ISubscriptionPublishPipeline : ISubscription`, implemented
**explicitly** by `Subscription` (the
`ISubscriptionMonitoredItemLifecycle` shape) so they are unreachable
through the public concrete class too. `Publish` goes with them
deliberately — it is the same hazard class (consumes notifications,
advances sequence numbers) with the same single production caller.
- **Kept public (1):** `ResendData` drops out of the issue's list — its
only caller is the `Server_ResendData` UA-method handler in
`DiagnosticsNodeManager`, resolving via `TryGetSubscription` like
`GetMonitoredItems` does. It is service-facing, not pipeline.

## The pipeline contract is complete: SessionPublishQueue traffics only
in it

`SessionPublishQueue` becomes `internal sealed` (zero consumers outside
the assembly; `SentMessageQueue` already was) and works **solely with
`ISubscriptionPublishPipeline`** — no `ISubscription` and no concrete
`Subscription` anywhere in the file. To get there, the internal
transfer-claim protocol (`TryBeginTransfer`,
`PrepareSessionTransferAsync`, `CompleteTransfer`, `AbortTransfer`,
`TryRestoreSessionAfterFailedTransfer`) moved from `internal` methods on
the concrete class onto the internal interface, implemented explicitly.

That completes the contract, so `TransferSubscriptionsAsync` loses its
two-tier shape: the ~6 `subscription as Subscription` null-guard
branches and the degraded `TransferSessionAsync` fallback for
non-`Subscription` implementations are gone — one code path, always the
claim/prepare/commit/rollback protocol, and
`ISubscription.TransferSessionAsync` is deleted with it. Subscription
creation fails with `Bad_InternalError` when a `CreateSubscription`
override returns a type not derived from `Subscription`, instead of
publishing partially — such implementations were already second-class
(transfer required the concrete type before this change). This is the
headline behavior change for exotic extenders and is documented
prominently in `docs/migrate/2.0.x/sessions-subscriptions.md`.

The durable-subscription open question in the issue is answered **no**:
`Subscription/Persistence/` never references `ISubscription` (the store
deals in `IStoredSubscription` DTOs; restore constructs the concrete
type via `Subscription.RestoreAsync` inside the manager), so the
internalization cannot break a store implementer.

## #4186: measured, prototyped, and deliberately kept as-is

Body inspection showed only 4 of the 10 `ISubscriptionManager` wrappers
are pure lookup-and-delegate (`Republish`, `SetTriggering`,
`ModifyMonitoredItemsAsync`, `SetMonitoringModeAsync` — one caller
each); the other 6 carry logic with no `ISubscription` substitute. A
collapse of the four was implemented and then reverted on this branch:
it traded ten uniform id-addressed members for a mixed dispatch shape in
`StandardServer` without reducing what a reader must understand. **Every
subscription service keeps going through `ISubscriptionManager`**; the
wrapper bodies now resolve against the pipeline-typed registry. The
decision is recorded in `plans/ServerRuntimeAnalysis.md` (open question
5).

## Compatibility treatment

Removals are outright, per the documented no-`[Obsolete]`-shim policy
for these interfaces (`docs/MigrationGuide.md` — re-adding an interface
member would break every implementer; net48/netstandard2.0 have no
default interface methods). In its place:

- Migration analyzer **UA0030** (pipeline members +
`SessionPublishQueue`, distinguishing deleted no-ops from internalized
protocol), with tests, following the UA0024 syntax-based pattern. The
transfer-claim members were never public in 1.5.378, so they need no
migration story.
- `docs/MigrationGuide.md#ua0030` section with before/after code; the
`docs/migrate/2.0.x/sessions-subscriptions.md` section that told
implementers to *add* `SessionClosed(ISession)` is replaced with the
derive-from-`Subscription` story; the 2.0.x README symptom table routes
the rule.
- All 11 removed/internalized members existed in 1.5.378 (verified
against `master378`), so each is documented as a 2.0 break.

## Tests

Converted, not deleted. `Mock<ISubscription>` setups of pipeline members
become `Mock<ISubscriptionPublishPipeline>` — Moq proxies the internal
interface via a new `InternalsVisibleTo("DynamicProxyGenAssembly2")`
(keyless; the SDK appends `$(PublicKey)` under `SignAssembly`, and
signed pipelines do not run tests, matching the existing
`Opc.Ua.Redundancy.Kubernetes` precedent). A test-side extension shim
forwards the original member names (pipeline + transfer protocol) so the
direct call sites on concrete `Subscription` instances compile
unchanged. The transfer-race tests' typed reflection probes into
`m_abandonedSubscriptions` follow the field's new type.

- `Opc.Ua.Server.Tests` (net10.0): 4795 tests, 0 failures
- `Opc.Ua.Subscriptions.Tests`, `Opc.Ua.Subscriptions.Durable.Tests`,
`Opc.Ua.Subscriptions.Classic.Tests` (net10.0): green
- `Opc.Ua.MigrationAnalyzer.Tests`: green incl. the new UA0030 tests
- net48 gate: solution build green; subscription-area fixtures green
(the known pre-existing local hosting-test environment failures
reproduce identically on unmodified master)


## Related Issues

- Fixes #4185
- #4186 was measured and prototyped on this branch and deliberately
**not** taken (see the section above); the decision is recorded in
`plans/ServerRuntimeAnalysis.md`.

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. _(net10
suites fully green locally; net48 build green and subscription-area
fixtures green — the remaining net48 hosting-test failures on my machine
reproduce identically on unmodified master.)_
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-31 10:53:51 +00:00
Copilot d525bc3f28 Bound reconnect cleanup and organize samples by domain (#4276)
# Description

This PR fixes bounded ManagedSession reconnect cleanup and reorganizes
the sample applications into domain-focused folders.

## Reconnect cleanup

- Bounds reconnect integration cleanup with independent 30-second close
and dispose limits.
- Sends `CloseSession` through the awaited transport request path during
`Session.DisposeAsync`.
- Preserves explicit `Session.CloseAsync` behavior that waits for or
cancels outstanding publish requests.
- Covers the disposal request path with a regression test.

## Sample organization

- `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer`
- `samples/Redundancy`: `RedundantClient`, `RedundantServer`,
`RedundantPubSub`
- `samples/Isa95`: `MinimalIsa95Server`
- `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`,
`MinimalClient`
- `samples/DI`: `PumpDeviceIntegrationServer`
- `samples/Lds`: `ConsoleLdsServer`
- `samples/Reference`: `ConsoleReferenceClient`,
`ConsoleReferenceServer`
- `samples/PubSub`: `ConsoleReferencePubSubClient`

`UA.slnx` now exposes the same requested domain groups. Project
references, tests, runtime sample probes, CI workflows, Dockerfiles,
Compose files, helper scripts, and documentation were updated to the new
paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`.

## Validation

- `dotnet build UA.slnx`
- 3 Redundancy short-haul sample tests
- 70 DI sample tests
- 914 OpenUSD tests
- 1 WotCon model-materialization test
- 137 AOT tests
- `docker compose config` for all 6 moved Redundancy Compose files

## Related Issues

- Fixes #4273

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce new build or
analyzer warnings.
- [x] I built the complete `UA.slnx` solution locally.
- [ ] I ran all tests locally against at least .NET Framework and .NET
10.
- [ ] I fixed all failing and flaky tests in the CI pipelines and all
CodeQL warnings.
- [x] I have addressed all PR feedback received.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <mschier@microsoft.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636
2026-08-21 04:35:47 +00:00
Marc Schier e05a820c91 Removal of the exposed diagnostics and node locks (#4188)
## What this is

Design analysis for `Opc.Ua.Server`, the two rounds of design
exploration behind it, and the first two implementation chunks it
recommends.

Draft: the analyses are ready to read, but the *conclusions* deserve
discussion before the larger phases proceed. Later chunks stack onto
this PR as they land.

| File | Kind | Lines |
|---|---|---:|
| `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin
surface | 488 |
| `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime
cluster | 576 |
| `plans/ServerDesignExploration.md` | Design round 2 — delivery
strategy | 1,394 |
| `plans/ServerDesignExplorationAlt.md` | Design round 1 — four
interface designs | 2,119 |
| `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards
attributes | |
| `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value
accessors take the lock | |
| `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** —
6 external locks removed | |
| `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** —
concurrency safety net | 449 |

`docs/` is untouched — it holds shipped feature documentation, and none
of this is shipped API.

## The analyses

Counted mechanically over `src`, `samples` and `tests`; **every figure
is reproducible** from commands printed in each document.

**Node manager surface.** `AsyncCustomNodeManager` declares **93
`virtual`/`abstract` members** (80 distinct names). Across the
repository: **112 override sites, 25 distinct names ever overridden, 55
of 80 never**. Six carry nearly all real use. For scale,
`ReferenceNodeManager` is 6,373 lines and overrides **8**.

**Server runtime.** Seven findings, **two of which recommend no
change**. Findings 1–5 are filed as #4183–#4187. The headline is an
`object`-typed sync root published on three public interfaces and taken
in **88 `lock` statements across 7 files**, including a shipped sample.

**What is deliberately not flagged.** `IStandardServer` — 9 members over
5,575 lines, 16 of 37 virtuals used — is identified as the reference
shape to preserve. `IApplicationInstance` and the already-`[Obsolete]`
`Stop()` are examined and left alone.

## The design exploration

**Round 1** explored four alternative interfaces in parallel under the
current back-compat constraint. Its value is the convergence: four
designers who never saw each other's work independently arrived at a 6–9
member `IServerContext` replacing the 57-member locator, removal of the
`object` node-manager handle, elimination of the `Processed` protocol,
and `IHistorianProvider` as the pattern to copy.

**Round 2** re-runs it with back-compat lifted and asks how to deliver:
full rewrite versus side-by-side fork. **Both option designers
independently declined to recommend their own assignment.** Three
structural facts were verified:

1. **The fork line already sits at `IAsyncNodeManager`** — old and new
node managers can coexist with zero runtime changes, so a fork
duplicates ~10% of the assembly, not all of it.
2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest
auto-fix ceiling for the authoring surface is **~10%**, against ~70% for
the shipped analyzers.
3. **A second lock leak exists** — `NodeState` takes `lock (this)` under
a `CA2002` suppression whose own comment documents the contract.

Recommendation: **neither pure option**, but a staged path reaching the
rewrite end state incrementally, gated on migrating the five
framework-internal node managers that today make it impossible for the
old base to ever freeze.

## Implementation: NodeState now guards its own attributes

This removes the prose-only contract in which `NodeState` took
`lock(this)` and every external caller was required to take
`lock(source)` on the node instance. The code already carried a TODO
describing exactly this fix and noting it required updating every
external site.

- The node self-guards rather than relying on callers.
`BaseVariableState` gains a **fully private** `System.Threading.Lock`
covering its value fields; `NodeState`'s other collections keep their
existing private locks.
- All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions
are gone.
- `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the
lock too — without that, a direct property write raced an attribute read
once the external lock was gone.
- **6 of 9** external sites removed. Five were already redundant
(`GetReferences`, `GetChildren`, `ReferenceExists` and
`SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state
via `GetChildren`); the sixth was an attribute read now covered by the
node's own lock.
- Attribute read/write deliberately take **no** node-wide lock: both
dispatch to caller-supplied handlers (`OnReadValue` and siblings) that
may take locks of their own, and holding a lock on the node across one
gives the node no fixed place in the lock order.
- **3 deliberately retained and annotated in code**: two `CreateBrowser`
calls, which are not internally synchronized (tracked in #4216), and one
in `CustomNodeManager`, whose class is slated for deletion so
refactoring its lock would be wasted work.

### No lock is exposed anywhere, to anyone

An earlier revision put the attribute lock on `NodeState` as `private
protected`, which still published a lock-sharing contract on the
inheritance surface. Review pushed back and it is gone:
`BaseVariableState` was the only user, so the lock moved there as a
plain `private readonly Lock`. There is no member for a derived type to
call, in this assembly or any other.

`BaseVariableValue` was the last public leak — it handed out the lock
guarding its value through a `public Lock` property, so any caller could
hold it across a call back into the stack. That property is removed:

- Derived value classes (what the source generator emits for every
structure variable) synchronize through a `protected EnterLock()` /
`ExitLock()` pair. Callback helpers were the first choice, but the
generated bodies pass `ref Variant` / `ref StatusCode` / `ref
DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The
lock *object* stays private either way.
- `NodeStateTemplates` emits the new form.
- `ServerInternalData`, the one cross-assembly caller, was reading the
lock back off a value it had itself constructed with that lock. It now
takes `m_diagnosticsLock` directly.

`NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a
subclass extending `Next()` legitimately needs its enumeration state
atomic with the base browser's, so the fix needs a base-owned
acquisition rather than a deletion.

### Migration coverage for the removed lock members

None of the removed lock members get an `[Obsolete]` shim, and the
migration guide now says why: a shim would have to be `object`-typed to
keep old call sites compiling, and on .NET 9+ `lock (o)` on a
statically-`object` reference is `Monitor.Enter`, which does **not**
interlock with `Lock.Enter()`. It would compile, run, and silently
synchronize nothing — strictly worse than a compile error. Re-adding a
member to a downstream-implemented interface is also a break in its own
right on `net48` / `netstandard2.0`, where there are no default
interface methods.

Instead, every removed member has a named replacement on the migration
path:

| Rule | Covers | Replacement it names |
|---|---|---|
| `UA0024` | `IServerInternal` / `ISession` / `ISubscription`
`DiagnosticsLock`, `DiagnosticsWriteLock` |
`UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`,
`ReadServerDiagnostics` / `ReadDiagnostics` |
| `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller
owns; the node guards its own state |
| `UA0026` | `BaseVariableValue.Lock` | a lock passed to the
`BaseVariableValue` constructor |

Each rule fires on sources still written against 1.5.378, where the
compiler only says a member is missing. `docs/MigrationGuide.md` gains a
section per removal with before/after.

### The tests earned their keep

The four concurrency tests were landed **first**, against the unmodified
code, because removing a synchronization contract cannot be verified by
the compiler and the existing `NodeStateHandlerConcurrencyTests` covers
a different concern (racing a handler assignment against a write, not
reader/writer exclusion). They now run with **no external locking**,
which is what demonstrates the node guards itself.

They caught two real problems, and one of them was mine:

- The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a
genuine race.
- One test asserted an invariant the design does not offer. Two property
setters are two lock acquisitions, so that pair was **never** atomic; it
only passed originally because the test held a lock across both. It now
uses a single attribute write carrying a whole `DataValue`, which
genuinely is atomic and is the path the server uses.

A third issue was found and fixed before it shipped: the reader loops
gated on `!writer.IsCompleted`, so a reader scheduled after the writer
finished performed zero reads — failing two runs in three on `net48`.
Converted to `do…while`; now five consecutive green runs.

**Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the
concurrency category green 5/5 consecutive on `net48`;
`Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205
passed, including `NodeManagerLifecycleTests` and
`AsyncCustomNodeManagerTests` which exercise the changed paths.

## Two corrections to my own published figures

Both were found while starting implementation, and both are recorded on
#4183.

**`ForceDiagnosticsScan()` was overstated.** The analysis claimed the
`DiagnosticsWriteLock` getter *"triggers a full address-space
diagnostics rescan"*. It is in fact a single flag assignment
(`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the
periodic timer. The defect stands — a side-effecting property getter
with a racy write on a synchronisation path — but its cost was
misstated.

**The contract was 15 sites, not 19.** A naive search matched ten
comment and `#pragma` lines that *mention* `lock(source)` while
explaining it. The documents now carry a counting command that excludes
comment matches.

A third finding expanded scope the other way: the `object`-typed lock
leak is **repo-wide**, with ten such members across `Opc.Ua.Core`,
`Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this`
and `ApplicationConfiguration.PropertiesLock => m_properties`, the same
defect class as `Subscription.DiagnosticsLock`.

## Caveats stated in the documents

- **Nothing in the designs is implemented or benchmarked.** Every
performance claim across all seven designs is a design intention, not a
measurement.
- The larger phases are a **major undertaking against a live plugin
API**; the documents say so rather than letting the neatness of the
proposals imply otherwise.
- The recommended sequencing puts low-risk, self-contained steps first
and notes they are **worth doing regardless** of which strategy is
eventually chosen.
- Round 2 notes the two option designers' cost estimates are **not
independent of their assignments**.

## Review guidance

The two analyses make factual claims checkable by running the included
commands. The design explorations are exploratory and need not be agreed
with to merge. The code change is small, test-backed, and independent of
which delivery strategy is eventually chosen.

Reasonable outcomes include merging as-is, merging only the analyses and
the lock fix, or disagreeing with the recommendation while keeping the
evidence.

---

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
2026-08-10 06:47:50 +02:00