diff --git a/CHANGELOG.md b/CHANGELOG.md index 52006a8..c2a0e3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,20 @@ Consequences: ### Fixed +- **A long-lived client no longer exhausts the host's operation quota** (#196). Every command + run in a shell holds one of the user's WSMan operations until the shell is deleted, even once + terminated, and the client reused its shell forever: a client polling indefinitely ended up + with every command refused with WSManFault 2150859174 (*the maximum number of concurrent + operations for this user has been exceeded*), after 15 commands on Windows Server 2008 R2 and + 1500 on later versions, until it was recreated. The client now replaces its shell every 10 + commands (the new `WinRMClient.Builder.maxCommandsPerShell(int)` changes that number) and never + reuses a shell holding a command it could not terminate (its terminate `Signal` failed or was + skipped). On Windows Server 2008 R2, whose quota fault carries its WSManFault code, a command + the quota refuses in a shell that already ran commands is also retried once in a new shell; + later versions send that fault with no code, so nothing reliable identifies it. The new shell + gets the same working directory, environment and profile; deleting the old one ends any process + a previous command left running in it, as closing the client always did. + - **A streaming read resumed after a long pause no longer times out spuriously** (#198). When a streaming consumer (e.g. a `RemoteProcess` read slowly) paused longer than the inactivity timeout and the host dropped the idle connection meanwhile, the reconnection inherited the diff --git a/src/main/java/org/metricshub/winrm/CommandRequest.java b/src/main/java/org/metricshub/winrm/CommandRequest.java index 604d2e4..832917a 100644 --- a/src/main/java/org/metricshub/winrm/CommandRequest.java +++ b/src/main/java/org/metricshub/winrm/CommandRequest.java @@ -192,8 +192,9 @@ private String powerShellFromFile(final String script, final long timeoutMillis, /** * Set the working directory of the remote process. The remote command shell is created on - * the first command a client executes and is reused afterward, so this setting takes effect - * only when this is the client's first command. + * the first command a client executes and is reused afterward (every shell that replaces it + * gets the same settings), so this setting takes effect only when this is the client's first + * command. * * @param workingDirectory the working directory path on the remote host * @return this request @@ -209,7 +210,8 @@ public CommandRequest workingDirectory(final String workingDirectory) { * be called several times; insertion order is preserved, and setting the same name again * replaces its value. Like {@link #workingDirectory(String)}, the environment is shell-scoped: * the remote command shell is created on the first command a client executes and is reused - * afterward, so this setting takes effect only when this is the client's first command. + * afterward (every shell that replaces it gets the same settings), so this setting takes + * effect only when this is the client's first command. * *
{@code
 	 * CommandResult result = client.command("build.cmd")
diff --git a/src/main/java/org/metricshub/winrm/ShellFileCopy.java b/src/main/java/org/metricshub/winrm/ShellFileCopy.java
index 417d558..67da651 100644
--- a/src/main/java/org/metricshub/winrm/ShellFileCopy.java
+++ b/src/main/java/org/metricshub/winrm/ShellFileCopy.java
@@ -118,8 +118,10 @@ private ShellFileCopy() {}
 
 	/**
 	 * Base delay before retrying after an operation-quota rejection; each retry waits one step
-	 * longer. Measured on Windows 2008 R2 (quota 15 per user): the budget fully recovers within
-	 * 30 seconds, so the escalating delays (5+10+15+20 s) comfortably bridge it.
+	 * longer (5+10+15+20 s). Time does not release the operations a shell holds, only
+	 * deleting the shell does, and the client already replaces its own shell when the quota
+	 * refuses a command (see {@code WsmanClient}): these delays give the user's other
+	 * connections time to release theirs.
 	 */
 	static final long QUOTA_RETRY_DELAY_MILLIS = 5_000L;
 
@@ -796,8 +798,9 @@ private static WindowsRemoteCommandResult run(
 
 				// The quota rejection happened while the operation was being CREATED — before the
 				// command could run — so retrying cannot duplicate a side effect. Old Windows
-				// versions cap concurrent operations very low (15 per user on 2008 R2) and reap
-				// completed ones lazily: give the server increasingly more time to recover.
+				// versions cap concurrent operations very low (15 per user on 2008 R2), and the
+				// client already replaced its own shell: give the user's other connections
+				// increasingly more time to release theirs.
 				try {
 					Utils.sleep(
 						Math.min(
diff --git a/src/main/java/org/metricshub/winrm/WinRMClient.java b/src/main/java/org/metricshub/winrm/WinRMClient.java
index e49daed..7bbd2aa 100644
--- a/src/main/java/org/metricshub/winrm/WinRMClient.java
+++ b/src/main/java/org/metricshub/winrm/WinRMClient.java
@@ -331,6 +331,7 @@ public static final class Builder {
 		private int consoleCodePage;
 		private boolean loadUserProfile;
 		private String arraySeparator = LightWinRMService.DEFAULT_ARRAY_SEPARATOR;
+		private int maxCommandsPerShell = LightWinRMService.DEFAULT_MAX_COMMANDS_PER_SHELL;
 		private SSLContext sslContext;
 		private Duration timeout = DEFAULT_TIMEOUT;
 		private int retries;
@@ -624,6 +625,38 @@ public Builder arraySeparator(final String arraySeparator) {
 			return this;
 		}
 
+		/**
+		 * Set how many commands the remote command shell runs before the client replaces it.
+		 * Default: 10.
+		 * 

+ * The client reuses its command shell for commands, file transfers and remote file + * operations, but every command run in a shell holds one of the user's WSMan operations + * until the shell is deleted, even after it completed. The host caps them per user, + * across all of that user's connections ({@code MaxConcurrentOperationsPerUser}: 15 on + * Windows Server 2008 R2, 1500 later), so a shell reused forever ends up having every + * command refused. Replacing the shell (one Delete and one Create, about 100 ms) releases + * them. The new shell gets the same working directory, environment and profile, but + * deleting the old one ends any process a previous command left running in it, as + * {@link WinRMClient#close()} does. Besides, on Windows Server 2008 R2, whose quota fault + * carries its WSManFault code, a command the quota refuses in a shell that already ran + * commands is retried once in a new shell; in a fresh shell, which holds nothing to + * release, the fault is reported. Later versions send that fault with no code, so this + * setting is what keeps a client under the quota there. + *

+ * A lower value leaves more of the quota to the user's other connections at the cost of + * more frequent replacements; 1 runs every command in a shell of its own, like {@code winrs}. + * + * @param maxCommandsPerShell how many commands a shell runs before it is replaced (at least 1) + * @return this builder + */ + public Builder maxCommandsPerShell(final int maxCommandsPerShell) { + if (maxCommandsPerShell < 1) { + throw new IllegalArgumentException("maxCommandsPerShell must be at least 1."); + } + this.maxCommandsPerShell = maxCommandsPerShell; + return this; + } + /** * Build the client. This does not connect yet: the connection is established and * authenticated by the first operation. @@ -672,6 +705,7 @@ public WinRMClient build() { consoleCodePage, loadUserProfile, arraySeparator, + maxCommandsPerShell, retries, toMillis(retryDelay) ); diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 828f80a..862e972 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -62,6 +62,13 @@ public final class LightWinRMService implements WindowsRemoteExecutor { /** The default string joining the elements of a WMI array property in a WQL row. */ public static final String DEFAULT_ARRAY_SEPARATOR = "|"; + /** + * The default number of commands a remote command shell runs before the client replaces it. + * Each command holds one of the user's WSMan operations until its shell is deleted, and + * Windows Server 2008 R2 allows only 15 per user ({@code MaxConcurrentOperationsPerUser}). + */ + public static final int DEFAULT_MAX_COMMANDS_PER_SHELL = 10; + private final WinRMEndpoint winRMEndpoint; private final WsmanClient client; private final AtomicBoolean closed = new AtomicBoolean(false); @@ -275,6 +282,71 @@ public static LightWinRMService createInstance( * Create a light WinRM executor that may delegate the caller's Kerberos credentials to the host, * so remote commands can authenticate onward as the caller (the second hop), may load the * user profile in the command shell, and joins WMI array properties with a custom separator. + * The command shell is replaced every {@link #DEFAULT_MAX_COMMANDS_PER_SHELL} commands. + * + * @param winRMEndpoint endpoint with credentials (mandatory) + * @param timeout timeout in milliseconds (must be > 0) + * @param ticketCache Kerberos ticket cache path (used by the Kerberos scheme; {@code null} logs + * in with the password) + * @param authentications requested authentication schemes, tried in order (NTLM, Kerberos, and/or Basic); + * {@code null}/empty means NTLM only + * @param allowDelegation whether Kerberos forwards the caller's ticket-granting ticket to the + * host (which must then be forwardable); requires Kerberos among {@code authentications} + * @param sslContext the {@link SSLContext} providing the HTTPS socket factory (hostname + * verification stays on); {@code null} uses the default configuration + * @param trustAllCertificates when {@code true} (and no {@code sslContext} is given), trust every + * server certificate and skip hostname verification — insecure, testing only + * @param consoleCodePage the console code page of the command shell; 0 keeps the default 65001, + * which makes command output UTF-8 whatever the remote locale + * @param loadUserProfile whether the command shell loads the user profile (registry hive, + * per-user environment variables); {@code false} is the historical behavior + * @param arraySeparator the string joining the elements of a WMI array property in a WQL row; + * see {@link #DEFAULT_ARRAY_SEPARATOR} + * @param connectRetries how many times one round trip may re-attempt to connect and authenticate + * (must be >= 0); 0 keeps the historical fail-fast behavior + * @param retryDelay the pause in milliseconds before each retry (must be >= 0) + * @return a new {@code LightWinRMService} + * @throws WinRMException on invalid arguments or an unsupported authentication request + */ + // CPD-OFF — a compatibility overload: its parameter list is the next overload's minus the + // shell bound, and reordering the parameters to fool the detector would break callers. + public static LightWinRMService createInstance( + final WinRMEndpoint winRMEndpoint, + final long timeout, + final java.nio.file.Path ticketCache, + final List authentications, + final boolean allowDelegation, + final SSLContext sslContext, + final boolean trustAllCertificates, + final int consoleCodePage, + final boolean loadUserProfile, + final String arraySeparator, + final int connectRetries, + final long retryDelay + ) throws WinRMException { + return createInstance( + winRMEndpoint, + timeout, + ticketCache, + authentications, + allowDelegation, + sslContext, + trustAllCertificates, + consoleCodePage, + loadUserProfile, + arraySeparator, + DEFAULT_MAX_COMMANDS_PER_SHELL, + connectRetries, + retryDelay + ); + // CPD-ON + } + + /** + * Create a light WinRM executor that may delegate the caller's Kerberos credentials to the host, + * so remote commands can authenticate onward as the caller (the second hop), may load the + * user profile in the command shell, joins WMI array properties with a custom separator, and + * replaces the command shell after a given number of commands. * * @param winRMEndpoint endpoint with credentials (mandatory) * @param timeout timeout in milliseconds (must be > 0) @@ -294,6 +366,8 @@ public static LightWinRMService createInstance( * per-user environment variables); {@code false} is the historical behavior * @param arraySeparator the string joining the elements of a WMI array property in a WQL row; * see {@link #DEFAULT_ARRAY_SEPARATOR} + * @param maxCommandsPerShell how many commands a command shell runs before it is replaced (must + * be > 0); see {@link #DEFAULT_MAX_COMMANDS_PER_SHELL} * @param connectRetries how many times one round trip may re-attempt to connect and authenticate * (must be >= 0); 0 keeps the historical fail-fast behavior * @param retryDelay the pause in milliseconds before each retry (must be >= 0) @@ -311,12 +385,16 @@ public static LightWinRMService createInstance( final int consoleCodePage, final boolean loadUserProfile, final String arraySeparator, + final int maxCommandsPerShell, final int connectRetries, final long retryDelay ) throws WinRMException { Utils.checkNonNull(winRMEndpoint, "winRMEndpoint"); Utils.checkNonNull(arraySeparator, "arraySeparator"); Utils.checkArgumentNotZeroOrNegative(timeout, "timeout"); + if (maxCommandsPerShell < 1) { + throw new IllegalArgumentException("maxCommandsPerShell must be at least 1."); + } if (connectRetries < 0) { throw new IllegalArgumentException("connectRetries must not be negative."); } @@ -366,6 +444,7 @@ public static LightWinRMService createInstance( consoleCodePage, loadUserProfile, arraySeparator, + maxCommandsPerShell, connectRetries, retryDelay ); diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index 73ed7e5..867dae9 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -58,6 +59,11 @@ final class WsmanClient implements AutoCloseable { private static final String FAULT_OPERATION_TIMEOUT = "2150858793"; private static final String FAULT_SHELL_NOT_FOUND = "2150858843"; + // The user's MaxConcurrentOperationsPerUser quota is full. Only Windows Server 2008 R2 sends this + // code: later versions send the fault with no WSManFault code and a generic InternalError + // subcode, which nothing reliable distinguishes from other faults (measured on 2016 and 2022). + private static final String FAULT_OPERATION_QUOTA = "2150859174"; + // A Send answered with this Windows error (ERROR_NO_DATA, "The pipe is being closed") found the // command no longer reading its standard input: it exited, or closed its stdin, first. private static final String FAULT_PIPE_CLOSING = "232"; @@ -92,6 +98,7 @@ final class WsmanClient implements AutoCloseable { private final int consoleCodePage; private final boolean loadUserProfile; private final String arraySeparator; + private final int maxCommandsPerShell; private final String url; private final String rawUsername; private final AuthScheme auth; @@ -107,6 +114,23 @@ final class WsmanClient implements AutoCloseable { private String pendingAuthorization; private String shellId; + // A shell no longer reused: it ran maxCommandsPerShell commands, the quota refused a command in + // it, or one of its commands may never have been terminated (its terminate Signal faulted, failed + // in transit, or found no budget). Its commands hold the user's WSMan operations until it is + // deleted, so reusing it would pile them up until MaxConcurrentOperationsPerUser refuses every + // command (issue #196). Deleted before the next shell is created, or by close(); never set + // together with shellId. Guarded by connectionPermit, like shellId. + private String retiredShellId; + + // How many commands the current shell has run. Each one holds one of the user's WSMan operations + // until the shell is deleted, even once cleanly terminated (measured on Windows Server 2008 R2 + // and 2022, issue #196), so the shell is retired after maxCommandsPerShell of them. Guarded by + // connectionPermit, like shellId. + @SuppressFBWarnings(value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", justification = "Only accessed while holding connectionPermit, " + + + "whose release/acquire orders every write before the next holder's reads") + private int shellCommands; + // The shell's working directory and environment variables are pinned by the FIRST command on // this connection and reused whenever the shell must be (re)created — e.g. after the server // reaped it — so a recreation stays invisible to the caller instead of silently moving later @@ -191,6 +215,7 @@ private static void checkNotCancelled() throws InterruptedException { final int consoleCodePage, final boolean loadUserProfile, final String arraySeparator, + final int maxCommandsPerShell, final int connectRetries, final long retryDelayMs ) { @@ -198,6 +223,7 @@ private static void checkNotCancelled() throws InterruptedException { this.consoleCodePage = consoleCodePage; this.loadUserProfile = loadUserProfile; this.arraySeparator = arraySeparator; + this.maxCommandsPerShell = maxCommandsPerShell; this.connectRetries = connectRetries; this.retryDelayMs = retryDelayMs; // A non-null socket factory selects HTTPS: TLS wraps the transport and the SOAP travels plaintext. @@ -559,7 +585,18 @@ RemoteCommand startCommand( : new LinkedHashMap<>(environment); shellSettingsPinned = true; } + if (shellId != null && shellCommands >= maxCommandsPerShell) { + // The shell's commands hold as many of the user's WSMan operations: replace it before + // they exhaust the quota shared by every connection of this user. + retireShell(); + } if (shellId == null) { + if (retiredShellId != null) { + deleteRetiredShell(operationTimeoutMs); + // The Delete was a round trip of its own: never create a shell after the caller's + // timeout was reported. + checkNotCancelled(); + } createShell(shellWorkingDirectory, shellEnvironment, operationTimeoutMs, failOnQuietTimeout); } // The caller's timeout may have fired while the Create response was being awaited (socket @@ -569,18 +606,26 @@ RemoteCommand startCommand( try { commandId = sendCommand(commandLine, operationTimeoutMs, failOnQuietTimeout, consoleModeStdin); } catch (final WinRMFaultException e) { - if (!FAULT_SHELL_NOT_FOUND.equals(e.getFaultCode())) { + // Either way the Command was rejected before it could run, so it is safe to recreate + // the shell — with its ORIGINAL working directory and environment — and retry once. + if (FAULT_SHELL_NOT_FOUND.equals(e.getFaultCode())) { + // The server reaped the cached shell between commands (e.g. its IdleTimeout expired on + // a long-lived client). + shellId = null; + } else if (shellCommands > 0 && isQuotaFault(e)) { + // The user's operation quota is full, and this shell holds one operation per command + // it ran: deleting it releases them. + retireShell(); + deleteRetiredShell(operationTimeoutMs); + checkNotCancelled(); + } else { throw e; } - // The server reaped the cached shell between commands (e.g. its IdleTimeout expired on a - // long-lived client). The Command was rejected before it could run, so it is safe to - // recreate the shell — with its ORIGINAL working directory and environment — and retry - // once. - shellId = null; createShell(shellWorkingDirectory, shellEnvironment, operationTimeoutMs, failOnQuietTimeout); checkNotCancelled(); commandId = sendCommand(commandLine, operationTimeoutMs, failOnQuietTimeout, consoleModeStdin); } + shellCommands++; opened = true; return new RemoteCommand(commandId, operationTimeoutMs, failOnQuietTimeout); } finally { @@ -925,15 +970,21 @@ private void finish() throws Exception { * Terminate a command closed before it completed: the Signal is what stops it, so its * failures are reported — except the expiry of its short hold (see * {@link #EARLY_CLOSE_SIGNAL_MS}), a complete exchange that leaves the connection in sync - * and the command killed. + * and the shell retired. Any failure, that expiry included, retires the shell (see + * {@link #retireShell()}): the fault only says the service did not finish processing the + * Signal in time, so the command may still be running in it. */ private void terminateRunning() throws Exception { try { terminate(commandId, Math.min(EARLY_CLOSE_SIGNAL_MS, operationTimeoutMs)); } catch (final WinRMFaultException e) { + retireShell(); if (!FAULT_OPERATION_TIMEOUT.equals(e.getFaultCode())) { throw e; } + } catch (final Exception e) { + retireShell(); + throw e; } } @@ -962,23 +1013,27 @@ private void finishBounded(final long budgetMs) { * complete, in-sync exchange and is simply ignored; any other failure (a timeout, a reset, * a half-read response) leaves the connection in an unknown state, so it is dropped — a * late response must not desync a later request. A budget too small for any round trip - * skips the Signal outright, leaving the healthy connection untouched; the server reaps - * the completed command's state with the shell. + * skips the Signal outright, leaving the healthy connection untouched. Whenever the Signal + * does not go through, the command keeps holding a WSMan operation until its shell is + * deleted, so the shell is retired (see {@link #retireShell()}) — never reused. */ private void terminateCompleted(final long budgetMs) { // Same strict clamp as the bounded poll: the per-round-trip timeout caps this cleanup too. final long budget = Math.max(1, Math.min(budgetMs, operationTimeoutMs)); if (budget < MIN_WIRE_POLL_MS) { + retireShell(); return; } transport.pollTimeout(toSocketTimeoutMillis(budget)); try { terminate(commandId, budget); - } catch (final WinRMFaultException ignored) { + } catch (final WinRMFaultException e) { // The Signal was answered with a fault: the exchange completed, the connection is in // sync — and the command's completion is what matters. + retireShell(); } catch (final Exception e) { transport.close(); + retireShell(); } finally { transport.inactivityTimeout(toSocketTimeoutMillis(operationTimeoutMs)); } @@ -1012,6 +1067,7 @@ private void createShell( final Element selector = (Element) selectors.item(i); if ("ShellId".equals(selector.getAttribute("Name"))) { shellId = selector.getTextContent(); + shellCommands = 0; return; } } @@ -1059,6 +1115,43 @@ private void signal(final String commandId, final String code, final long timeou } } + /** + * Stop reusing the current shell: only deleting it releases the WSMan operations its commands + * hold (issue #196). The Delete is deferred to the next shell creation or to {@link #close()}: + * a failed or skipped Signal leaves no budget, and possibly no connection, to send it now. + */ + private void retireShell() { + if (shellId != null) { + retiredShellId = shellId; + shellId = null; + } + } + + /** + * Best-effort Delete of the retired shell, sent right before its replacement is created. No + * failure of it may fail the new command, which owes nothing to its predecessor's cleanup: a + * shell this Delete cannot reach is left to the server's IdleTimeout. A failure other than a + * WSMan fault drops the connection, so the Create that follows starts on a fresh, + * re-authenticated one: a response rejected for its HTTP status is never decrypted, which + * leaves the message encryption out of sync on this connection. + */ + private void deleteRetiredShell(final long timeoutMs) { + final String shell = retiredShellId; + retiredShellId = null; + try { + request(Envelopes.deleteShell(url, shell, timeoutMs)); + } catch (final InterruptedException e) { + // Cancelled in a connect-retry pause, before the Delete was sent: keep the shell retired so + // a later command or close() still deletes it. The restored interrupt makes the caller + // abort before its Create, so the shell is never retired alongside a new one. + retiredShellId = shell; + Thread.currentThread().interrupt(); + } catch (final Exception e) { + // best-effort shell cleanup, on a connection whose state is now unknown + transport.close(); + } + } + // --- transport / crypto ------------------------------------------------- /** Send a request, expecting HTTP 200; throw with the WSMan fault detail otherwise. */ @@ -1394,6 +1487,11 @@ private static String wsmanFaultCode(final Document doc) { return faults.getLength() > 0 ? ((Element) faults.item(0)).getAttribute("Code") : null; } + /** Whether the fault says the user's WSMan operation quota (MaxConcurrentOperationsPerUser) is full. */ + private static boolean isQuotaFault(final WinRMFaultException fault) { + return FAULT_OPERATION_QUOTA.equals(fault.getFaultCode()); + } + /** * The detailed WSManFault Message text, or null. This is where WinRM puts the provider-level * detail — notably the WMI error mnemonics (WBEM_E_INVALID_CLASS, WBEM_E_INVALID_NAMESPACE, @@ -1452,8 +1550,10 @@ public void close() { // transport — which unblocks that worker's read; the shell is reaped by the server IdleTimeout. final boolean locked = connectionPermit.tryAcquire(); try { - final String shell = shellId; + // At most one of the two is set: a retired shell is deleted before the next one is created. + final String shell = shellId != null ? shellId : retiredShellId; shellId = null; + retiredShellId = null; if (locked && shell != null) { try { send(Envelopes.deleteShell(url, shell, timeoutMs)); diff --git a/src/site/markdown/commands.md b/src/site/markdown/commands.md index 0bb624a..bd7cfc4 100644 --- a/src/site/markdown/commands.md +++ b/src/site/markdown/commands.md @@ -75,7 +75,8 @@ try (WinRMClient client = WinRMClient.builder("server.example.com") The profile is loaded when the remote shell is created, so this is a client setting. It applies to every shell the client creates, for commands, file transfers and remote file operations alike, -including a shell recreated after the server reaped the previous one. Microsoft's `winrs` +including a shell recreated after the server reaped the previous one or the client replaced it +(see [Shell reuse](#shell-reuse)). Microsoft's `winrs` documentation warns that loading the profile fails for a user who is not a local administrator on the host: the command then fails with a [`WinRMFaultException`](apidocs/org/metricshub/winrm/exceptions/WinRMFaultException.html) carrying @@ -85,6 +86,30 @@ A command that reaches a further host (a UNC path, another server) fails with *a unless the client delegates your Kerberos credentials: see [Credential delegation](authentication.html#credential-delegation). +### Shell reuse + +The client runs its commands, file transfers and remote file operations in one remote command +shell, created by the first of them. But every command run in a shell holds one of the user's +WSMan operations until the shell is deleted, even after it completed, and the host caps them per +user, across all of that user's connections: `MaxConcurrentOperationsPerUser` is 15 on Windows +Server 2008 R2 and 1500 later (see [Host quotas](preparing-the-host.html#host-quotas-worth-knowing-about)). +So the client replaces its shell: + +* every 10 commands. The builder's `maxCommandsPerShell(int)` changes that number: lower leaves + more of the quota to the user's other connections, and 1 runs every command in a shell of its + own, like `winrs`; +* on Windows Server 2008 R2, when the quota refuses a command in a shell that already ran + commands: the command ran nothing, so it is retried once in a new shell. In a fresh shell, which + holds nothing to release, the fault is reported. Later versions send the quota fault without its + WSManFault code, so nothing reliable identifies it: there, replacing the shell every N commands + is what keeps the client under the quota; +* when a command could not be terminated cleanly (its terminate `Signal` failed or was skipped). + +A replacement costs a Delete and a Create (about 100 ms). The new shell gets the same working +directory, environment variables and profile, but deleting the old one ends any process a previous +command left running in it, as closing the client does. A process that must outlive its command +belongs outside the shell, e.g. in a scheduled task. + ## Running PowerShell `powerShell(...)` prepares a PowerShell script execution the same way `command(...)` prepares a diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index 7a2b761..104b0d0 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -151,6 +151,7 @@ Besides the host name passed to `builder(...)`, only `credentials(...)` is manda | `retries(int, Duration)` | no retry | [Retrying](timeouts-and-errors.html#retrying-transient-connection-failures) | | `namespace(String)` | `ROOT\CIMV2` | [Choosing a namespace](wql.html#choosing-a-namespace) | | `loadUserProfile()` | not loaded | [Loading the user profile](commands.html#loading-the-user-profile) | +| `maxCommandsPerShell(int)` | 10 | [Shell reuse](commands.html#shell-reuse) | | `consoleCodePage(int)` | 65001 (UTF-8) | [Input encoding](commands.html#input-encoding) | | `arraySeparator(String)` | `\|` | [Reading the result](wql.html#reading-the-result) | diff --git a/src/site/markdown/migrating-from-winrm4j.md b/src/site/markdown/migrating-from-winrm4j.md index 0f87d40..639d758 100644 --- a/src/site/markdown/migrating-from-winrm4j.md +++ b/src/site/markdown/migrating-from-winrm4j.md @@ -158,12 +158,15 @@ the switch: to retry `Receive` after an operation timeout. Here nothing is retried unless you opt in with `retries(int, Duration)`, and the policy is deliberately narrow: only attempts that provably never reached the server (TCP connect, DNS, TLS handshake, the authentication handshake) are - retried, preserving **at-most-once execution** for non-idempotent commands. A request that was - actually sent is never replayed. -* **One shell per client.** winrm4j creates and tears down a remote shell for every - `executeCommand(...)`. This client creates the shell on the first command and reuses it, which - is faster — and is why `workingDirectory(...)` and `environment(...)` are per-command options - that take effect on the client's **first** command ([command options](commands.html#command-options)). + retried, preserving **at-most-once execution** for non-idempotent commands. A request that may + have run is never replayed: the only replays are of a command the host refused before running it + (its shell was reaped, or the quota was full; see [Shell reuse](commands.html#shell-reuse)). +* **One shell for many commands.** winrm4j creates and tears down a remote shell for every + `executeCommand(...)`. This client creates the shell on the first command and reuses it, for 10 + commands by default ([Shell reuse](commands.html#shell-reuse)), which is faster — and is why + `workingDirectory(...)` and `environment(...)` are per-command options that take effect on the + client's **first** command, then on every shell that replaces it + ([command options](commands.html#command-options)). ## What you gain diff --git a/src/site/markdown/preparing-the-host.md b/src/site/markdown/preparing-the-host.md index b56afac..90474e5 100644 --- a/src/site/markdown/preparing-the-host.md +++ b/src/site/markdown/preparing-the-host.md @@ -369,7 +369,7 @@ the host, the tighter they are: | --- | --- | --- | | `MaxMemoryPerShellMB` | Memory per shell, including child processes | Historically **150 MB**; 1024 MB on modern hosts. A command whose output is large can hit it. | | `MaxShellsPerUser` | Concurrent shells per user | 5 on older hosts, 30 on modern ones. Close clients you no longer need. | -| `MaxConcurrentOperationsPerUser` | Concurrent operations per user | 15 on Windows Server 2008 R2, 1500 later. File transfers are batched specifically to stay under low limits. | +| `MaxConcurrentOperationsPerUser` | Concurrent operations per user | 15 on Windows Server 2008 R2, 1500 later. File transfers are batched specifically to stay under low limits. Every command holds one until its shell is deleted, even after it completed: the client replaces its shell every 10 commands (and, on 2008 R2, when the quota refuses a command in a shell that already ran commands), so a long-lived client does not pile them up (see [Shell reuse](commands.html#shell-reuse)). | | `MaxEnvelopeSizekb` | SOAP envelope size | 150 KB on older hosts, **500 KB** on modern ones. This client always uses 150 KB envelopes, so the default never needs raising. | | `IdleTimeout` | How long an idle shell survives | 180000 ms (3 min) on older hosts, **7200000 ms** (2 h) on modern ones; 60000 ms minimum. The client transparently recreates a shell the host reaped. | diff --git a/src/test/java/org/metricshub/winrm/StreamingApiTest.java b/src/test/java/org/metricshub/winrm/StreamingApiTest.java index 831a360..489ba8c 100644 --- a/src/test/java/org/metricshub/winrm/StreamingApiTest.java +++ b/src/test/java/org/metricshub/winrm/StreamingApiTest.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.done; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellDeletion; import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; import static org.metricshub.winrm.light.FakeWsmanResponses.enumerationDone; import static org.metricshub.winrm.light.FakeWsmanResponses.fault; @@ -784,6 +785,96 @@ void completionInsideATinyPollSkipsTheSignal() throws Exception { } } + @Test + void aSkippedCompletionSignalRetiresTheShell() throws Exception { + enqueueCommandStartup(); + server.enqueue(200, envelope(receiveResponse(stdoutChunk("done\n"), done(COMMAND_ID, 5)))); + enqueueNextCommandInANewShell(); + + try (WinRMClient client = builder().build()) { + try (CommandCursor cursor = client.executor().startCommand("run.exe", null, 10_000)) { + cursor.poll(5_000); + // No round trip fits the budget: the Signal is skipped, the command never terminated. + assertNull(cursor.poll(20)); + } + // Issue #196: reusing SHELL-1 would leave that command holding a WSMan operation for good. + assertNextCommandRunsInANewShell(client); + } + } + + @Test + void aFailedEarlyCloseSignalRetiresTheShell() throws Exception { + enqueueCommandStartup(); + // The Signal stopping the still-running command is rejected: the failure is reported, and + // the command may still be running in SHELL-1. + server.enqueue(500, fault("999", "Signal rejected")); + enqueueNextCommandInANewShell(); + + try (WinRMClient client = builder().build()) { + final CommandCursor cursor = client.executor().startCommand("run.exe", null, 10_000); + assertThrows(WinRMClientException.class, cursor::close); + assertNextCommandRunsInANewShell(client); + } + } + + @Test + void anEarlyCloseSignalHeldPastItsHoldRetiresTheShell() throws Exception { + enqueueCommandStartup(); + // The service does not finish processing the Signal within its short hold: not a failure of + // close(), but nothing proves the command is gone from SHELL-1. + server.enqueue(500, fault(FAULT_OPERATION_TIMEOUT, "The operation timed out.")); + enqueueNextCommandInANewShell(); + + try (WinRMClient client = builder().build()) { + client.executor().startCommand("run.exe", null, 10_000).close(); + assertNextCommandRunsInANewShell(client); + } + } + + @Test + void anEarlyCloseSignalLostInTransitRetiresTheShell() throws Exception { + enqueueCommandStartup(); + // The connection drops before the Signal is answered: whether the command was stopped is + // unknown, so SHELL-1 must not be reused either. + server.enqueueDrop(); + enqueueNextCommandInANewShell(); + + try (WinRMClient client = builder().build()) { + final CommandCursor cursor = client.executor().startCommand("run.exe", null, 10_000); + assertThrows(WinRMClientException.class, cursor::close); + assertNextCommandRunsInANewShell(client); + } + } + + /** Script the next command: the retired shell's Delete, then a whole command in SHELL-2. */ + private void enqueueNextCommandInANewShell() { + enqueueShellDeletion(server); + server + .enqueue(200, envelope(resourceCreated("SHELL-2"))) + .enqueue(200, envelope(commandResponse("CMD-2"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-2", "two".getBytes(StandardCharsets.UTF_8)), done("CMD-2", 0))) + ) + .enqueue(200, envelope(signalResponse())); + } + + /** Run the next command: it must delete SHELL-1 and run in a fresh SHELL-2, never reuse SHELL-1. */ + private void assertNextCommandRunsInANewShell(final WinRMClient client) throws Exception { + final int first = server.decryptedRequests().size(); + try (CommandCursor cursor = client.executor().startCommand("next.exe", null, 10_000)) { + assertEquals("two", new String(cursor.next().stdout(), StandardCharsets.UTF_8)); + assertNull(cursor.next()); + } + final List requests = server.decryptedRequests(); + final String delete = requests.get(first); + assertTrue(delete.contains("transfer/Delete"), delete); + assertTrue(delete.contains("Selector Name=\"ShellId\">SHELL-1<"), delete); + assertTrue(requests.get(first + 1).contains("transfer/Create"), requests.get(first + 1)); + final String command = requests.get(first + 2); + assertTrue(command.contains("Selector Name=\"ShellId\">SHELL-2<"), command); + } + @Test void commandSilenceBeyondTheTimeoutSurfacesAsInactivityTimeout() throws Exception { enqueueCommandStartup(); diff --git a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java index 31e8e93..7925e95 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java @@ -99,6 +99,16 @@ void retriesMustBeNonNegativeWithANonNegativeDelay() { } } + @Test + void maxCommandsPerShellMustBeAtLeastOne() { + assertThrows(IllegalArgumentException.class, () -> validBuilder().maxCommandsPerShell(0)); + assertThrows(IllegalArgumentException.class, () -> validBuilder().maxCommandsPerShell(-1)); + // 1 is valid: every command in a shell of its own. + try (WinRMClient client = validBuilder().maxCommandsPerShell(1).build()) { + assertEquals("host", client.hostname()); + } + } + @Test void authenticationMustNotBeEmpty() { assertThrows(IllegalArgumentException.class, () -> validBuilder().authentication()); diff --git a/src/test/java/org/metricshub/winrm/WinRMClientTest.java b/src/test/java/org/metricshub/winrm/WinRMClientTest.java index 653db7d..b89ff5c 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientTest.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.done; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellDeletion; import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; import static org.metricshub.winrm.light.FakeWsmanResponses.enumerationDone; import static org.metricshub.winrm.light.FakeWsmanResponses.fault; @@ -67,6 +68,12 @@ class WinRMClientTest { private static final String WSEN = "http://schemas.xmlsoap.org/ws/2004/09/enumeration"; private static final String WSMAN = "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd"; + /** The quota fault, as Windows Server 2008 R2 sends it: the only version that carries its code. */ + private static final String QUOTA_FAULT = fault( + "2150859174", + "The WS-Management service cannot process the request. The maximum number of concurrent operations for this user has been exceeded." + ); + private FakeWsmanServer server; @BeforeEach @@ -901,4 +908,143 @@ void closedClientRejectsOperationsAndCloseIsIdempotent() { client.close(); assertThrows(IllegalStateException.class, () -> client.wql("SELECT Name FROM Win32_Service").execute()); } + + // --- Shell replacement (issue #196) ------------------------------------------- + // + // Every command run in a shell holds one of the user's WSMan operations until the shell is + // deleted, even once cleanly terminated (measured on Windows Server 2008 R2 and 2022): a shell + // reused forever ends up having every command refused by MaxConcurrentOperationsPerUser. + + /** Script a whole command printing the given output, in the current shell. */ + private void enqueueCommand(final String commandId, final String stdout) { + server + .enqueue(200, envelope(commandResponse(commandId))) + .enqueue( + 200, + envelope( + receiveResponse(stream("stdout", commandId, stdout.getBytes(StandardCharsets.UTF_8)), done(commandId, 0)) + ) + ) + .enqueue(200, envelope(signalResponse())); + } + + /** Script the Delete of the replaced shell, then the creation of its replacement. */ + private void enqueueShellReplacement(final String newShell) { + enqueueShellDeletion(server); + server.enqueue(200, envelope(resourceCreated(newShell))); + } + + /** Assert that the requests from the given index delete SHELL-1, create a shell and use SHELL-2. */ + private void assertShellReplacedAt(final int index) { + assertShellReplacedAt(index, "SHELL-1", "SHELL-2"); + } + + /** Assert that the requests from the given index delete the old shell, create one and use the new one. */ + private void assertShellReplacedAt(final int index, final String oldShell, final String newShell) { + final List requests = server.decryptedRequests(); + final String delete = requests.get(index); + assertTrue(delete.contains("transfer/Delete"), delete); + assertTrue(delete.contains("Selector Name=\"ShellId\">" + oldShell + "<"), delete); + assertTrue(requests.get(index + 1).contains("transfer/Create"), requests.get(index + 1)); + final String command = requests.get(index + 2); + assertTrue(command.contains("Selector Name=\"ShellId\">" + newShell + "<"), command); + } + + @Test + void theShellIsReplacedAfterMaxCommandsPerShell() throws Exception { + server.enqueue(200, envelope(resourceCreated("SHELL-1"))); + enqueueCommand("CMD-1", "first"); + enqueueCommand("CMD-2", "second"); + enqueueShellReplacement("SHELL-2"); + enqueueCommand("CMD-3", "third"); + enqueueCommand("CMD-4", "fourth"); + enqueueShellReplacement("SHELL-3"); + enqueueCommand("CMD-5", "fifth"); + + try (WinRMClient client = builder(PASSWORD).maxCommandsPerShell(2).build()) { + for (final String output : List.of("first", "second", "third", "fourth", "fifth")) { + assertEquals(output, client.command(output + ".exe").execute().stdout()); + } + } + + // Create, then two whole commands (Command, Receive, Signal) in SHELL-1... + assertShellReplacedAt(7); + // ...and the count starts over in SHELL-2: two more commands before the next replacement. + assertShellReplacedAt(15, "SHELL-2", "SHELL-3"); + } + + @Test + void theShellIsReplacedAfterTenCommandsByDefault() throws Exception { + server.enqueue(200, envelope(resourceCreated("SHELL-1"))); + for (int i = 1; i <= 10; i++) { + enqueueCommand("CMD-" + i, "out"); + } + enqueueShellReplacement("SHELL-2"); + enqueueCommand("CMD-11", "out"); + + try (WinRMClient client = builder(PASSWORD).build()) { + for (int i = 0; i < 11; i++) { + assertEquals("out", client.command("poll.exe").execute().stdout()); + } + } + + // Create, then ten whole commands (Command, Receive, Signal) in SHELL-1. + assertShellReplacedAt(31); + } + + @Test + void aCommandRefusedByTheQuotaIsRetriedInANewShell() throws Exception { + server.enqueue(200, envelope(resourceCreated("SHELL-1"))); + enqueueCommand("CMD-1", "first"); + // The user's operation quota is full: the second Command is refused before it could run. + server.enqueue(500, QUOTA_FAULT); + enqueueShellReplacement("SHELL-2"); + enqueueCommand("CMD-2", "second"); + + try (WinRMClient client = builder(PASSWORD).build()) { + assertEquals("first", client.command("first.exe").execute().stdout()); + // SHELL-1 holds an operation per command it ran: deleting it releases them, and the + // refused command runs in a new shell. + assertEquals("second", client.command("second.exe").execute().stdout()); + } + + // Create, the first command, the refused Command in SHELL-1, then the replacement. + final String refused = server.decryptedRequests().get(4); + assertTrue(refused.contains("Selector Name=\"ShellId\">SHELL-1<"), refused); + assertShellReplacedAt(5); + } + + @Test + void aQuotaFaultOnAFreshShellIsReported() throws Exception { + // The new shell ran nothing: deleting it would release nothing, so the fault is reported. + server.enqueue(200, envelope(resourceCreated("SHELL-1"))).enqueue(500, QUOTA_FAULT); + + try (WinRMClient client = builder(PASSWORD).build()) { + final WinRMFaultException e = assertThrows(WinRMFaultException.class, () -> client.command("x.exe").execute()); + assertEquals("2150859174", e.getFaultCode()); + assertEquals(2, server.decryptedRequests().size(), "no Delete, no retry"); + } + } + + @Test + void aCommandRefusedByTheQuotaIsRetriedOnlyOnce() throws Exception { + // The quota is held by other connections of the user: the new shell is refused too, and + // the fault is reported instead of looping. + server.enqueue(200, envelope(resourceCreated("SHELL-1"))); + enqueueCommand("CMD-1", "first"); + server.enqueue(500, QUOTA_FAULT); + enqueueShellReplacement("SHELL-2"); + server.enqueue(500, QUOTA_FAULT); + + try (WinRMClient client = builder(PASSWORD).build()) { + assertEquals("first", client.command("first.exe").execute().stdout()); + final WinRMFaultException e = assertThrows( + WinRMFaultException.class, + () -> client.command("second.exe").execute() + ); + assertEquals("2150859174", e.getFaultCode()); + final long commands = server.decryptedRequests().stream().filter(r -> r.contains(":CommandLine>")).count(); + assertEquals(3, commands, "the first command, the refused one, and a single retry"); + } + } } diff --git a/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java b/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java index 2cb8216..4964128 100644 --- a/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java +++ b/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java @@ -49,6 +49,7 @@ void closeWhileOperationInFlightStillErasesTheBasicCredential() throws Exception 65001, false, "|", + LightWinRMService.DEFAULT_MAX_COMMANDS_PER_SHELL, 0, 0L ); diff --git a/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java b/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java index 1ffc664..5237146 100644 --- a/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java +++ b/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.done; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellDeletion; import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; import static org.metricshub.winrm.light.FakeWsmanResponses.fault; import static org.metricshub.winrm.light.FakeWsmanResponses.instance; @@ -61,6 +62,7 @@ class WsmanProtocolTest { private static final String WSEN = "http://schemas.xmlsoap.org/ws/2004/09/enumeration"; private static final String WSMAN = "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd"; private static final String RSP = "http://schemas.microsoft.com/wbem/wsman/1/windows/shell"; + private static final String DELETE_RESPONSE = ""; private FakeWsmanServer server; @@ -408,6 +410,124 @@ void terminateSignalToleratesShellNotFoundFault() throws Exception { } } + @Test + void aFaultedTerminateSignalRetiresTheShell() throws Exception { + // Issue #196: a command whose terminate Signal failed keeps holding one of the user's WSMan + // operations until its shell is deleted. A long-lived client that kept reusing the shell piled + // them up until MaxConcurrentOperationsPerUser refused every Command. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-1", "one".getBytes(StandardCharsets.UTF_8)), done("CMD-1", 0))) + ) + .enqueue(500, fault("999", "Signal rejected")); + enqueueNextCommandInANewShell(); + + assertNextCommandRunsInANewShell(); + } + + @Test + void aTerminateSignalLostInTransitRetiresTheShell() throws Exception { + // Same as a faulted Signal, but the connection drops before the answer: whether the command + // was terminated is unknown, so its shell must not be reused either. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-1", "one".getBytes(StandardCharsets.UTF_8)), done("CMD-1", 0))) + ) + .enqueueDrop(); + enqueueNextCommandInANewShell(); + + assertNextCommandRunsInANewShell(); + } + + @Test + void closeDeletesARetiredShell() throws Exception { + // The last command's Signal failed, and no other command follows to delete its shell: close() + // must. A client per collection cycle would otherwise leave a shell and its held operation behind + // every cycle. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-1", "one".getBytes(StandardCharsets.UTF_8)), done("CMD-1", 0))) + ) + .enqueue(500, fault("999", "Signal rejected")); + enqueueShellDeletion(server); + + try (LightWinRMService service = client(PASSWORD)) { + assertEquals("one", service.executeCommand("poll", null, StandardCharsets.UTF_8, TIMEOUT).getStdout()); + } + + final List requests = server.decryptedRequests(); + final String delete = requests.get(requests.size() - 1); + assertTrue(delete.contains("transfer/Delete"), delete); + assertTrue(delete.contains("Selector Name=\"ShellId\">SHELL-1<"), delete); + } + + @Test + void aRetiredShellDeleteAnsweredWithAnUnexpectedStatusDoesNotFailTheNextCommand() throws Exception { + // The best-effort Delete draws an HTTP 503 whose sealed body is never decrypted: reusing that + // connection would decrypt the Create's response with an out-of-sync cipher stream. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-1", "one".getBytes(StandardCharsets.UTF_8)), done("CMD-1", 0))) + ) + .enqueue(500, fault("999", "Signal rejected")) + .enqueue(503, envelope(DELETE_RESPONSE)); + enqueueCommandInShell2(); + + assertNextCommandRunsInANewShell(); + } + + /** Script the second command: the retired shell's Delete, then a whole command in SHELL-2. */ + private void enqueueNextCommandInANewShell() { + enqueueShellDeletion(server); + enqueueCommandInShell2(); + } + + /** Script a whole command in a freshly created SHELL-2. */ + private void enqueueCommandInShell2() { + server + .enqueue(200, envelope(resourceCreated("SHELL-2"))) + .enqueue(200, envelope(commandResponse("CMD-2"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-2", "two".getBytes(StandardCharsets.UTF_8)), done("CMD-2", 0))) + ) + .enqueue(200, envelope(signalResponse())); + } + + /** + * Run two commands on one client, the first one's terminate Signal failing: both must succeed, + * and the second must delete SHELL-1 and run in a fresh SHELL-2 instead of reusing SHELL-1. + */ + private void assertNextCommandRunsInANewShell() throws Exception { + try (LightWinRMService service = client(PASSWORD)) { + // The failed Signal is cleanup noise: the completed command's result is still reported. + assertEquals("one", service.executeCommand("poll", null, StandardCharsets.UTF_8, TIMEOUT).getStdout()); + assertEquals("two", service.executeCommand("poll", null, StandardCharsets.UTF_8, TIMEOUT).getStdout()); + } + + final List requests = server.decryptedRequests(); + // Create, Command, Receive, the failed Signal, then the second command's requests. + assertTrue(requests.size() >= 9, () -> String.join("\n---\n", requests)); + final String delete = requests.get(4); + assertTrue(delete.contains("transfer/Delete"), delete); + assertTrue(delete.contains("Selector Name=\"ShellId\">SHELL-1<"), delete); + assertTrue(requests.get(5).contains("transfer/Create"), requests.get(5)); + final String command = requests.get(6); + assertTrue(command.contains("Selector Name=\"ShellId\">SHELL-2<"), command); + } + // --- Fault mapping ------------------------------------------------------------ @Test diff --git a/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java b/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java index 372892b..135ae91 100644 --- a/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java +++ b/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java @@ -23,9 +23,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.metricshub.winrm.light.FakeWsmanResponses.COMMAND_ID; +import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; +import static org.metricshub.winrm.light.FakeWsmanResponses.done; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueCommandExchange; import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueEnumeration; import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellCreation; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellDeletion; +import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; import static org.metricshub.winrm.light.FakeWsmanResponses.instance; +import static org.metricshub.winrm.light.FakeWsmanResponses.receiveResponse; +import static org.metricshub.winrm.light.FakeWsmanResponses.resourceCreated; +import static org.metricshub.winrm.light.FakeWsmanResponses.stream; import java.net.ServerSocket; import java.nio.charset.StandardCharsets; @@ -263,6 +272,43 @@ void closeDuringARetryPauseAbortsInsteadOfRevivingTheConnection() throws Excepti } } + @Test + void aRetiredShellDeleteCancelledInARetryPauseIsSentByTheNextCommand() throws Exception { + // The first command's terminate Signal is lost in transit: SHELL-1 is retired (issue #196). + enqueueShellCreation(server); + server + .enqueue(200, envelope(commandResponse(COMMAND_ID))) + .enqueue( + 200, + envelope( + receiveResponse(stream("stdout", COMMAND_ID, "one".getBytes(StandardCharsets.UTF_8)), done(COMMAND_ID, 0)) + ) + ) + .enqueueDrop(); + + try (LightWinRMService service = service(server.port(), 3, 5_000L)) { + assertEquals("one", service.executeCommand("poll", null, StandardCharsets.UTF_8, TIMEOUT).getStdout()); + + // The second command's Delete must reconnect: the handshake is dropped, and the deadline + // cancels the command during the retry pause, before the Delete was ever sent. + server.dropNextConnections(1); + assertThrows(TimeoutException.class, () -> service.executeCommand("poll", null, StandardCharsets.UTF_8, 1_500L)); + + // SHELL-1 is still retired: the third command deletes it before creating SHELL-2. + enqueueShellDeletion(server); + server.enqueue(200, envelope(resourceCreated("SHELL-2"))); + enqueueCommandExchange(server, "two".getBytes(StandardCharsets.UTF_8), new byte[0], 0); + assertEquals("two", service.executeCommand("poll", null, StandardCharsets.UTF_8, TIMEOUT).getStdout()); + } + + final List requests = server.decryptedRequests(); + // Create, Command, Receive, the dropped Signal, then the third command's requests. + final String delete = requests.get(4); + assertTrue(delete.contains("transfer/Delete"), delete); + assertTrue(delete.contains("Selector Name=\"ShellId\">SHELL-1<"), delete); + assertTrue(requests.get(6).contains("Selector Name=\"ShellId\">SHELL-2<"), requests.get(6)); + } + // --- the wall-clock deadline still governs --------------------------------- @Test