mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 22:55:08 +08:00
Marc Schier
3713641479
# Description Fixes a busy-spin in the client publish worker that crashed the **net48** CI test host (e.g. [build 16092](https://opcfoundation.visualstudio.com/opcua-netstandard/_build/results?buildId=16092&view=results), `Test Release` reported *"The active test run was aborted. Reason: Test host process crashed"* on `SubscriptionManagerTests.PublishWorkerDefersDeletingUnknownSubscriptionWhileCreationIsPendingAsync`). It also carries a small, self-contained CI fix for the repository-wide CodeQL out-of-memory failure (see the last section). ## Root cause In `SubscriptionManager.PublishWorkerAsync`, a publish response whose `SubscriptionId` does not resolve to a subscription the manager owns set `moreNotifications = true`. That flag **skips the ack-wait throttle**, so the worker immediately re-issues `Publish`. The condition is *persistent* — the server keeps answering with the same undeliverable identifier — so the worker spun with **zero delay**, logged at `Information` level on every iteration, and in the orphan branch issued `DeleteSubscriptions` for the very same subscription on every single response. With a transport that completes synchronously the loop additionally never released its thread pool thread. Measured on the unmodified code, for a single 485 ms test: | Metric | Before | |---|---| | `DeleteSubscriptions` calls for one orphan | **16,500** | | "deferred unknown subscription" log lines | **4,296** | | TRX produced by that one test | **4.86 MB** (47% of the whole 2,124-test assembly) | | Peak test host working set | ~528 MB | On a slow 2-core hosted agent the polling loops stretch out, the spin runs far longer, and the unbounded log/allocation growth takes the test host down. Beyond the CI symptom this is a real client defect: against a live server the client would hammer it with `Publish`/`DeleteSubscriptions` and flood the log whenever a subscription identifier cannot be resolved. ## Changes `src/Opc.Ua.Client/Subscription/SubscriptionManager.cs` - Undeliverable publish responses (retired identifier, identifier still pending creation, or genuine orphan) now **always throttle** through a new `DelayUnresolvedSubscriptionAsync` — an escalating, bounded backoff (20 ms doubling to a 250 ms cap) — instead of republishing immediately. - After deleting an orphan, its identifier is passed to `RetireSubscriptionId`, so responses already in flight (or emitted before the delete takes effect) are ignored rather than triggering another delete. **One delete instead of thousands.** - The deferral message is logged once per run of the same unresolved identifier instead of once per response. - The unresolved-response counters reset as soon as a response is dispatched successfully. `tests/Opc.Ua.Client.Tests/Subscription/Fakes/FakeSubscriptionManagerContext.cs` - The call recordings were plain `List<T>` appended to from publish-worker threads while the test thread read them. They are now guarded by a `System.Threading.Lock` and exposed as snapshot `IReadOnlyList<T>` getters. - Added `DeleteCallsCount`, a locked count accessor, so polling wait conditions do not allocate a snapshot per iteration. `tests/Opc.Ua.Client.Tests/Subscription/SubscriptionManagerTests.cs` - `PublishWorkerThrottlesWhileSubscriptionIdIsUnresolvedAsync` — bounds the publish count over a 500 ms window; fails loudly on the old code. - `PublishWorkerDeletesUnknownSubscriptionOnlyOnceAsync` — asserts exactly one `DeleteSubscriptions` call for an orphan. ## Results | Metric (single test) | Before | After | |---|---|---| | Deferred log lines | 4,296 | 3 | | `DeleteSubscriptions` log lines | 16,500 | 3 | | TRX size | 4,856,749 B | 6,466 B (**750×** smaller) | The full-assembly TRX dropped from 10.30 MB to 4.92 MB. No public API change; behaviour change is limited to the undeliverable-publish-response path. ## CodeQL out-of-memory fix (`.github/workflows/codeql-analysis.yml`) The CodeQL job has been failing repository wide — on `master` and on every open PR — with: ``` CodeQL is out of memory. Try running CodeQL on a larger runner (hosted or self-hosted). Error running analysis for csharp: ... codeql database interpret-results --threads=4 ... Exit code was 99 ``` The CodeQL step builds `UA.slnx` without `CustomTestTarget`, so `targets.props` falls through to its default and multi-targets up to **six** frameworks (`net472`, `net48`, `netstandard2.1`, `net8.0`, `net9.0`, `net10.0`). CodeQL therefore extracts every source file once per framework. The same multiplier applies to source-generated code, which `EmitCompilerGeneratedFiles` materializes on disk — `Opc.Ua.Core.Types` alone emits **52 MB** of generated C# for a *single* framework. The database and source archive grew until `interpret-results` exhausted the runner's memory. - Pin the CodeQL build to `-p:CustomTestTarget=net10.0` so every source file is extracted once instead of ~6×. Verified locally: `UA.slnx` builds with **0 errors** under that property. - Limit the analyze step to `threads: 2`, bounding the peak memory of the step that failed. This is unrelated to the publish-worker fix and is happy to be split out if maintainers prefer. ## Review feedback addressed - Snapshot properties are read once into a local before asserting, so two reads cannot observe different snapshots. - Wait conditions poll the new non-allocating `DeleteCallsCount` instead of `DeleteCalls.Count`. - The sync root is declared as the fully qualified `System.Threading.Lock`. An `object`-typed sync root was **not** used: repository coding standards forbid it, and `Opc.Ua.Types/Polyfills/System.Threading.cs` already backports a public `System.Threading.Lock` for every TFM below net9.0. The branch was rebased onto current `master` (`8a10139b8`). ## Related Issues - No tracked issue; reported via the failing `Test Release` (net48) leg of build 16092. ## 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. _(Internal behaviour fix, no API or documented-behaviour change.)_ - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. _(0 warnings, 0 errors on net48 and net10.0.)_ - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - `Opc.Ua.Client.Tests` **net48** (CI filter): 2,124 passed, 0 failed - `Opc.Ua.Client.Tests` **net10.0**: 2,121 passed, 0 failed - `Opc.Ua.Subscriptions.Tests` **net10.0** (integration): 602 passed, 0 failed - `SubscriptionManagerTests` fixture run 5× on net48: 36/36 every run (no flakiness) - [x] I have addressed **all** PR feedback received. - [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6615d151-e4af-43c7-8a32-4531842e7461
77 lines
3.0 KiB
YAML
77 lines
3.0 KiB
YAML
# A CodeQL query for the UA .NET Standard project
|
|
name: "CodeQL"
|
|
|
|
on:
|
|
push:
|
|
branches: [ master, main, master378, release/*, develop/* ]
|
|
pull_request:
|
|
# The branches below must be a subset of the branches above
|
|
branches: [ master, main, master378 ]
|
|
paths:
|
|
- '**.cs'
|
|
schedule:
|
|
- cron: '30 6 * * 6'
|
|
workflow_dispatch:
|
|
|
|
permissions:
|
|
actions: read
|
|
contents: read
|
|
packages: read
|
|
security-events: write
|
|
|
|
concurrency:
|
|
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
|
|
cancel-in-progress: true
|
|
|
|
jobs:
|
|
analyze:
|
|
name: Analyze
|
|
runs-on: windows-2025
|
|
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
language: [ 'csharp' ]
|
|
# CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python' ]
|
|
# Learn more:
|
|
# https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#changing-the-languages-that-are-analyzed
|
|
|
|
steps:
|
|
- name: Checkout repository
|
|
uses: actions/checkout@v7
|
|
|
|
# Initializes the CodeQL tools for scanning.
|
|
- name: Initialize CodeQL
|
|
uses: github/codeql-action/init@v4
|
|
with:
|
|
languages: ${{ matrix.language }}
|
|
# If you wish to specify custom queries, you can do so here or in a config file.
|
|
# By default, queries listed here will override any specified in a config file.
|
|
# Prefix the list here with "+" to use these queries and those in the config file.
|
|
# queries: ./path/to/local/query, your-org/your-repo/queries@main
|
|
|
|
- name: Set up .NET
|
|
uses: actions/setup-dotnet@v6
|
|
with:
|
|
dotnet-version: '10.x'
|
|
# https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages#c-compiler-flags-injected-by-codeql-for-manual-builds
|
|
#
|
|
# CustomTestTarget pins the build to a single target framework (see
|
|
# targets.props). A default build multi-targets up to six frameworks, so
|
|
# CodeQL would extract every source file - and every source generated file,
|
|
# which EmitCompilerGeneratedFiles materializes on disk and which alone is
|
|
# tens of megabytes per project - once per framework. That duplication grew
|
|
# the database until "codeql database interpret-results" ran out of memory
|
|
# on the hosted runner. Analyzing one framework covers the same sources
|
|
# once and keeps the database within the runner's memory budget.
|
|
- name: Build Solution
|
|
run: |
|
|
dotnet build "UA.slnx" -c Release -p:CustomTestTarget=net10.0 -p:UseSharedCompilation=false -p:EmitCompilerGeneratedFiles=true
|
|
- name: Perform CodeQL Analysis
|
|
uses: github/codeql-action/analyze@v4
|
|
with:
|
|
# Interpreting results in parallel keeps a result set and its source
|
|
# snippets in memory per thread. Two threads bound the peak memory of
|
|
# the step that previously failed with an out of memory error.
|
|
threads: 2
|