Repository navigation
Issue #196: Retire the shell when a command's terminate Signal does not go through - #197
Conversation
A long-lived client polling periodically ended up with every command refused with WSManFault 2150859174 (maximum number of concurrent operations for this user exceeded), until the client was recreated. When the terminate Signal ending a command faulted, failed in transit, or was skipped for lack of budget in a bounded poll, the client kept reusing the shell, and the never-terminated command kept holding one of the user's WSMan operations until the shell was deleted, which only close() did. The same happened when the Signal stopping a still-running command failed. Such a shell is now retired: the next command deletes it and runs in a fresh shell created with the same working directory, environment and profile, and close() deletes it when no command follows. The Delete is deferred rather than sent right away because a completed command's cleanup must neither outlive the caller's poll budget nor fail it, and the failed or skipped Signal leaves no budget, and possibly no connection, for it. The command that follows restores its own timeouts after the Delete (a timed-out Delete must not leave the streaming per-leg deadline expired for the Create's reconnect) and checks for cancellation before creating the new shell. Closes #196 Co-Authored-By: Claude Opus 5.5 <[email protected]>
NassimBtk
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7b9e83dd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Address Codex review: a Delete answered with an unexpected HTTP status (e.g. 503) is rejected by send() before its sealed body is decrypted, while HttpTransport.post() keeps the socket open because the exchange completed. Swallowing that failure let the Create that follows reuse an NTLM session whose incoming cipher stream was out of sync, failing the new command with a checksum mismatch. Any failure of the best-effort Delete other than a WSMan fault now closes the transport, so the Create starts on a fresh, re-authenticated connection, as terminateCompleted() already does for the Signal. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7664221edb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address Codex review: with connection retries enabled, the deferred Delete of a retired shell can be cancelled by the command's wall-clock deadline during a connect-retry pause, before the request is sent. The retired shell ID was already cleared, so a later command created a new shell without retrying the cleanup, leaving the old command's operation allocated until the server's IdleTimeout. The ID is now restored on that cancellation path, so the next command or close() still deletes it; the restored interrupt makes the cancelled command abort before its Create, so the shell is never retired alongside a new one. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Address review: the OperationTimeout fault answering the terminate Signal of a command closed before completion only says the service did not finish processing the Signal within its 1 s hold, not that the command is gone. Keeping the shell bet on the command having been killed; if it was not, its operation stayed allocated, which is #196 on this path. The shell is now retired on that fault too: close() still succeeds, and the next command deletes the shell and creates a new one. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Since #198, HttpTransport.connect() re-arms the streaming deadline on every explicit reconnect, so the Create that follows a timed-out retired-shell Delete no longer inherits an expired deadline. Remove the configureTimeouts() call that worked around it (its comment was no longer true) and keep the cancellation check. Also restore the blank line between the #196 and #198 CHANGELOG entries. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Live test resultsTested on real hosts against The fix works, and nothing regressedThe probe uses the public API:
The cost of a retired shell: the next command takes about 100 ms longer for the Delete and the Create (55 → 155 ms on 2022, 40 → 130 ms on 2008 R2). But the operations also pile up when every Signal succeedsSame loop, but each command is drained with
So on a real host, every command run in a reused shell holds one WSMan operation until that shell is deleted, terminate Signal or not. A long-lived polling client reaches the limit after 1500 commands on a modern host, and after 15 on 2008 R2. Recreating the client is the only way out. That matches the symptom described in #196, and it happens without any failed Signal. The reproduction in the issue only used Side finding: on tc-win2022 (French), the quota fault is an HTTP 500 with no SuggestionThis PR is still correct for the paths it covers. But #196 needs the client to replace its shell on its own as well:
Either way, this PR's mechanism (retire now, Delete before the next Create or in |
Live tests on Windows Server 2008 R2 (MaxConcurrentOperationsPerUser = 15) showed the 16th command of a cycle failing with WSManFault 2150859174: the terminate Signal fails on that host after every command, and winrm-java#196 leaks one operation in the shell each time. A fresh client per command deletes its shell, and with it the leaked operation, after every command; a pooled command client piled them up over the cycle. Commands are back to one client each, as on main. WQL queries, the bulk of a cycle's requests, keep their pooled clients: they never open a remote shell, so a pooled WQL client leaves nothing on the host. That also removes the need for the WQL/command pool split and for the CLI shutdown hook. Pool commands too once winrm-java ships MetricsHub/winrm-java#197. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Address live tests on Windows Server 2008 R2 and 2022: every command run in a shell holds one of the user's WSMan operations until the shell is deleted, even when its terminate Signal succeeded; time does not release them. A long-lived client therefore reached MaxConcurrentOperationsPerUser (15 on 2008 R2, 1500 later) without any failed Signal, which is #196 too. - The client replaces its shell every maxCommandsPerShell commands: a new WinRMClient.Builder option, 10 by default, plumbed through a new LightWinRMService.createInstance overload (the previous one delegates with the default). WsmanClient counts the commands run in the current shell and retires it before the next command once the count is reached; the existing flow deletes it and creates a new one with the same working directory, environment and profile. - A Command the quota refuses in a shell that already ran commands retires and deletes that shell, then is retried once in a new one (it ran nothing). In a fresh shell the fault is reported. - The quota fault is recognized by its code 2150859174 or, on hosts that send no WSManFault element (a French Windows Server 2022), by its SOAP subcode, now exposed as WinRMFaultException.getFaultSubcode(). Docs: new "Shell reuse" section in commands.md (including that deleting a shell ends processes left running in it), client options row, fault detail table, host quota row, winrm4j migration guide, CHANGELOG. The ShellFileCopy quota-retry comments no longer claim the budget recovers over time. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Live test results on 0665a09Replacing the shell every N commands works
The quota retry only works on 2008 R2Test: client A, built with The quota fault, as each host sends it:
Why the every-N replacement should stay the main fix
The retry is still worth keeping for 2008 R2. With a quota of 15, two clients of the same user at up to 10 commands each can exceed it, and the fault code is present there to detect it. Suggestions
|
Address live tests on Windows Server 2008 R2, 2016 and 2022: no host sends a QuotaLimit subcode with the quota fault. 2008 R2 sends WSManFault code 2150859174 with the generic InternalError subcode; 2016 and 2022 send no WSManFault code at all, only InternalError, which cannot identify this fault. So: - The quota fault is recognized by its code only, which means the retry-in-a-new-shell runs on 2008 R2 only. Replacing the shell every maxCommandsPerShell commands stays the main fix, the only one that works on later versions. - WinRMFaultException.getFaultSubcode(), added for that match, is removed (back to main), with its fault detail table row. - The builder Javadoc, commands.md, preparing-the-host.md and the CHANGELOG scope the quota retry to Windows Server 2008 R2. - Tests: the subcode test and its made-up fixture are gone; the quota tests use the fault as 2008 R2 sends it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
2 participants
Closes #196
Problem
A long-lived
WinRMClientpolling periodically ends up with every command refused withHTTP 500 (WSManFault 2150859174): the maximum number of concurrent operations for this user has been exceeded. It never recovers until the client is recreated.The client reuses one remote shell for all its commands. Live tests on Windows Server 2008 R2 and 2022 (see the live test results comment) showed that every command run in a shell holds one of the user's WSMan operations until the shell is deleted, even when its terminate
Signalsucceeded, and that time does not release them.MaxConcurrentOperationsPerUseris 15 on 2008 R2 and 1500 later, shared by all connections of the user. Onlyclose()deleted the shell.On top of that, a command whose terminate
Signalfailed or was skipped stayed in the reused shell.Fix
The client replaces its shell (retires it, deletes it, creates a new one) in three cases:
maxCommandsPerShellcommandsWinRMClient.Builder.maxCommandsPerShell(int), default 10 (1 = a shell per command, likewinrs)The new shell gets the same pinned working directory, environment and profile. Deleting the old one ends any process a previous command left running in it, as
close()always did (documented).The retired shell is deleted right before the next shell is created, or by
close(). Its Delete is best effort and never fails the next command: a failure other than a WSMan fault drops the connection (a response rejected for its HTTP status is never decrypted, which would desync NTLM); if it is cancelled before being sent, the shell stays retired; no shell is created after the caller was told the command timed out.The quota fault is recognized by its WSManFault code
2150859174, which only Windows Server 2008 R2 sends. Measured on real hosts:2150859174InternalErrorInternalErrorInternalErrorFrom 2012 on, nothing reliable identifies the quota fault (only its translated reason text), so the quota retry runs on 2008 R2 only, and replacing the shell every N commands is the main fix. While one client fills the quota, the user's other connections are refused too, WQL included, which is another reason not to wait for the fault.
Tests
15 new tests against
FakeWsmanServer. Each fails when the line of the fix it guards is undone:WinRMClientTest: replacement every N commands (count restarting in each new shell), every 10 by default, quota retry, fault reported on a fresh shell, single retryWinRMClientBuilderTest:maxCommandsPerShellvalidationWsmanProtocolTest: faulted Signal (the issue's scenario), Signal lost in transit,close()deleting a retired shell, Delete answered with HTTP 503StreamingApiTest: Signal skipped in a tiny poll, early-close Signal failed, held past its hold, or lost in transitWsmanRetryTest: Delete cancelled in a connect-retry pausemvn verifypasses: 347 tests (9 skipped: live tests needing a Windows host), with no Checkstyle, PMD or SpotBugs findings.Docs
commands.md: new "Shell reuse" section;index.md: client options row;preparing-the-host.md: quota row;migrating-from-winrm4j.md: shell reuse and replay statements corrected;CHANGELOG.md: "Fixed" entry.ShellFileCopycomments no longer claim the quota budget recovers over time.Not in this PR
ShellFileCopy/RemoteFilesquota wait-and-retry still match the numeric code only; the client's own shell replacement now runs before them.Commandrequest whose response is lost can still leave an unknown command in the shell until its next replacement.🤖 Generated with Claude Code