Skip to content

[Server] Bound the server shutdown drain and make request admission an extension - #4148

Closed
marcschier wants to merge 1 commit into
marcschier/wot-05-lifecyclefrom
marcschier/wot-06-shutdown
Closed

[Server] Bound the server shutdown drain and make request admission an extension#4148
marcschier wants to merge 1 commit into
marcschier/wot-05-lifecyclefrom
marcschier/wot-06-shutdown

Conversation

@marcschier

Copy link
Copy Markdown
Collaborator

Summary

  • PR 6 of the stacked split of integration PR WoT Binding and WoT Connectivity 1.1 #4093.
  • Adds coverage for the RequestManager lifecycle extension, request-admission closure, closed-admission drain retry behavior, AsyncLocal request-scope nesting, and StandardServer double dispose.
  • Leaves subscription transfer and publish-queue lifecycle coverage for PR 7.

Stack context

This PR is stacked on #4147 (marcschier/wot-05-lifecycle) and intentionally needs its shutdown-progress members: MasterNodeManager.ShutdownCompletedNodeManagerCount and NodeManagerLifecycle.ShutdownCleanupProgress.

The shutdown hardening remains minimal and in-place because the maintainer chose that over extracting a shutdown coordinator. The AsyncLocal scope nesting is retained deliberately as a correctness fix; removing AsyncLocal is tracked separately.

Validation

  • dotnet build src\Opc.Ua.Server\Opc.Ua.Server.csproj -c Release -f net10.0 -v:m
  • dotnet build tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -p:CustomTestTarget=net10.0 -v:m
  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -p:CustomTestTarget=net10.0 --no-build — 3927 passed, 9 skipped
  • dotnet build tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -p:CustomTestTarget=net48 -v:m
  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -p:CustomTestTarget=net48 --no-build — 3926 passed, 9 skipped, 1 known local-only failure (DurableDataValueQueueVerifyReferenceBatchingAsync)
  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -p:CustomTestTarget=net48 --no-build --filter FullyQualifiedName~DurableDataValueQueueVerifyReferenceBatchingAsync — isolated known flake passed

Factors request admission control and NodeManager lifecycle waiter tracking out of RequestManager into an optional registerable extension, so the request manager itself carries only the generic request tracking and drain machinery. With no extension registered the behaviour is exactly as before.

The server-internal shutdown reserves its task slot under the coordination lock but starts the shutdown outside it, so no synchronous work runs under the lock. The deferred retry is bounded by a maximum attempt count and a time budget instead of retrying for as long as cleanup progress keeps increasing, and a terminal failure now surfaces through ServerError and a log rather than only being recorded on the shutdown state.

Final node-manager cleanup is deliberately not cancellable: cancelling it would leak retired lifecycle state, node managers and the server semaphore after request admission has already been closed permanently.

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

Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
@marcschier

Copy link
Copy Markdown
Collaborator Author

Closing this in favour of folding these tests into #4147.

While preparing this PR I verified that its three intended source files -
StandardServer.cs, RequestManager.cs and RequestManagerLifecycleExtension.cs -
are already byte-identical between marcschier/wot-05-lifecycle and the integration
branch. PR #4147 absorbed them when it took the NodeManagerLifecycle /
MasterNodeManager work, which needs the same shutdown drain. git diff between the
two branches is empty, so there is no source change left for this PR to carry.

What was genuinely missing was the test coverage for that shutdown and request-admission
work, which is what this branch ended up containing. Tests belong in the same PR as the
code they cover, so RequestManagerTests.cs and the StandardServerCoverageTests.cs
additions are being moved into #4147 instead of shipping as a separate PR.

No coverage is lost: these tests were validated on this branch at 3927 passing on
net10.0 and 3926 on net48, and they will be re-validated in #4147.

@marcschier marcschier closed this Jul 31, 2026
@marcschier
marcschier deleted the marcschier/wot-06-shutdown branch July 31, 2026 12:32
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.

1 participant