[exec] Prevent deadlock (#19152)

* Add TextOutputConsumer
* Add stdout and stderr channels
* Add charset configuration
* Add upgrade instructions

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

bundles/org.openhab.binding.exec/src/main/java/org/openhab/binding/exec/internal/ExecBindingConstants.java

@@ -30,6 +30,8 @@ public class ExecBindingConstants {
 
     // List of all Channel ids
     public static final String OUTPUT = "output";
+    public static final String STDOUT = "stdout";
+    public static final String STDERR = "stderr";
     public static final String INPUT = "input";
     public static final String EXIT = "exit";
     public static final String RUN = "run";

bundles/org.openhab.binding.exec/src/main/java/org/openhab/binding/exec/internal/handler/ExecHandler.java

@@ -18,16 +18,22 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.math.BigDecimal;
+import java.nio.charset.Charset;
+import java.nio.charset.IllegalCharsetNameException;
+import java.nio.charset.StandardCharsets;
+import java.nio.charset.UnsupportedCharsetException;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.IllegalFormatException;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.regex.PatternSyntaxException;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -82,6 +88,7 @@ public class ExecHandler extends BaseThingHandler {
     public static final String COMMAND = "command";
     public static final String TRANSFORM = "transform";
     public static final String AUTORUN = "autorun";
+    public static final String CHARSET = "charset";
 
     private ExecutorService executor;
     private @Nullable ScheduledFuture<?> scheduledTask;
@@ -172,18 +179,22 @@ public class ExecHandler extends BaseThingHandler {
             timeOut = ((BigDecimal) getConfig().get(TIME_OUT)).intValue() * 1000;
         }
 
+        Charset charset = null;
+        String charsetValue = (String) getConfig().get(CHARSET);
+        if (charsetValue != null && !charsetValue.isBlank()) {
+            try {
+                charset = Charset.forName(charsetValue);
+            } catch (IllegalCharsetNameException | UnsupportedCharsetException e) {
+                logger.warn("Invalid or unsupported character encoding '{}', falling back to UTF-8", charsetValue);
+            }
+        }
+        if (charset == null) {
+            charset = StandardCharsets.UTF_8;
+        }
+
         if (commandLine != null && !commandLine.isEmpty()) {
             updateState(RUN, OnOffType.ON);
 
-            // For some obscure reason, when using Apache Common Exec, or using a straight implementation of
-            // Runtime.Exec(), on Mac OS X (Yosemite and El Capitan), there seems to be a lock race condition
-            // randomly appearing (on UNIXProcess) *when* one tries to gobble up the stdout and sterr output of the
-            // subprocess in separate threads. It seems to be common "wisdom" to do that in separate threads, but
-            // only when keeping everything between .exec() and .waitfor() in the same thread, this lock race
-            // condition seems to go away. This approach of not reading the outputs in separate threads *might* be a
-            // problem for external commands that generate a lot of output, but this will be dependent on the limits
-            // of the underlying operating system.
-
             Date date = Calendar.getInstance().getTime();
             try {
                 if (lastInput != null) {
@@ -197,6 +208,8 @@ public class ExecHandler extends BaseThingHandler {
                         commandLine, date, lastInput, e.getMessage());
                 updateState(RUN, OnOffType.OFF);
                 updateState(OUTPUT, new StringType(e.getMessage()));
+                updateState(STDOUT, new StringType());
+                updateState(STDERR, new StringType(e.getMessage()));
                 return;
             }
 
@@ -210,6 +223,8 @@ public class ExecHandler extends BaseThingHandler {
                     logger.warn("An exception occurred while splitting '{}' : '{}'", commandLine, e.getMessage());
                     updateState(RUN, OnOffType.OFF);
                     updateState(OUTPUT, new StringType(e.getMessage()));
+                    updateState(STDOUT, new StringType());
+                    updateState(STDERR, new StringType(e.getMessage()));
                     return;
                 }
             } else {
@@ -235,6 +250,8 @@ public class ExecHandler extends BaseThingHandler {
                         logger.warn("OS {} not supported, please manually split commands!", getOperatingSystemName());
                         updateState(RUN, OnOffType.OFF);
                         updateState(OUTPUT, new StringType("OS not supported, please manually split commands!"));
+                        updateState(STDOUT, new StringType());
+                        updateState(STDERR, new StringType("OS not supported, please manually split commands!"));
                         return;
                 }
             }
@@ -254,13 +271,19 @@ public class ExecHandler extends BaseThingHandler {
                         e.getMessage());
                 updateState(RUN, OnOffType.OFF);
                 updateState(OUTPUT, new StringType(e.getMessage()));
+                updateState(STDOUT, new StringType());
+                updateState(STDERR, new StringType(e.getMessage()));
                 return;
             }
 
             StringBuilder outputBuilder = new StringBuilder();
             StringBuilder errorBuilder = new StringBuilder();
 
-            try (InputStreamReader isr = new InputStreamReader(proc.getInputStream());
+            TextOutputConsumer errConsumer = new TextOutputConsumer();
+            Future<List<String>> stdErrFuture = errConsumer.consume(proc.getErrorStream(), charset,
+                    Thread.currentThread().getName() + "-stderr-consumer");
+
+            try (InputStreamReader isr = new InputStreamReader(proc.getInputStream(), charset);
                     BufferedReader br = new BufferedReader(isr)) {
                 String line;
                 while ((line = br.readLine()) != null) {
@@ -272,18 +295,6 @@ public class ExecHandler extends BaseThingHandler {
                         e.getMessage());
             }
 
-            try (InputStreamReader isr = new InputStreamReader(proc.getErrorStream());
-                    BufferedReader br = new BufferedReader(isr)) {
-                String line;
-                while ((line = br.readLine()) != null) {
-                    errorBuilder.append(line).append("\n");
-                    logger.debug("Exec [{}]: '{}'", "ERROR", line);
-                }
-            } catch (IOException e) {
-                logger.warn("An exception occurred while reading the stderr when executing '{}' : '{}'", commandLine,
-                        e.getMessage());
-            }
-
             boolean exitVal = false;
             try {
                 exitVal = proc.waitFor(timeOut, TimeUnit.MILLISECONDS);
@@ -297,17 +308,43 @@ public class ExecHandler extends BaseThingHandler {
                 proc.destroyForcibly();
             }
 
+            try {
+                List<String> stdErr = stdErrFuture.get(timeOut, TimeUnit.MILLISECONDS);
+                for (String line : stdErr) {
+                    errorBuilder.append(line).append("\n");
+                    logger.debug("Exec [{}]: '{}'", "ERROR", line);
+                }
+            } catch (InterruptedException e) {
+                logger.debug("Interrupted while waiting for process ('{}') stderr content", commandLine);
+            } catch (TimeoutException e) {
+                logger.warn("Timed out while waiting for process ('{}') stderr content", commandLine);
+            } catch (ExecutionException e) {
+                Throwable cause = e.getCause();
+                logger.warn("An exception occurred while reading the stderr when executing '{}' : '{}'", commandLine,
+                        cause != null ? cause.getMessage() : e.getMessage());
+            }
+
             updateState(RUN, OnOffType.OFF);
             updateState(EXIT, new DecimalType(proc.exitValue()));
 
+            ChannelTransformation transformation = channelTransformation;
+            String transformedStdout = Objects.requireNonNull(StringUtils.chomp(outputBuilder.toString()));
+            String transformedStderr = Objects.requireNonNull(StringUtils.chomp(errorBuilder.toString()));
+            if (transformation != null) {
+                transformedStdout = transformation.apply(transformedStdout).orElse(transformedStdout);
+                transformedStderr = transformation.apply(transformedStderr).orElse(transformedStderr);
+            }
+            updateState(STDOUT, new StringType(transformedStdout));
+            updateState(STDERR, new StringType(transformedStderr));
+
             outputBuilder.append(errorBuilder.toString());
 
             outputBuilder.append(errorBuilder.toString());
 
             String transformedResponse = Objects.requireNonNull(StringUtils.chomp(outputBuilder.toString()));
 
-            if (channelTransformation != null) {
-                transformedResponse = channelTransformation.apply(transformedResponse).orElse(transformedResponse);
+            if (transformation != null) {
+                transformedResponse = transformation.apply(transformedResponse).orElse(transformedResponse);
             }
 
             updateState(OUTPUT, new StringType(transformedResponse));
@@ -344,6 +381,8 @@ public class ExecHandler extends BaseThingHandler {
                 logger.warn("An exception occurred while splitting '{}' : '{}'", commandLine, e.getMessage());
                 updateState(RUN, OnOffType.OFF);
                 updateState(OUTPUT, new StringType(e.getMessage()));
+                updateState(STDOUT, new StringType());
+                updateState(STDERR, new StringType(e.getMessage()));
                 return new String[] {};
             }
         }

bundles/org.openhab.binding.exec/src/main/java/org/openhab/binding/exec/internal/handler/TextOutputConsumer.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2010-2025 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.exec.internal.handler;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.FutureTask;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
+/**
+ * Spawns a new thread that consumes all bytes from the specified {@link InputStream}, converts
+ * the stream to characters using the specified {@link Charset} and then returns the content
+ * as a {@link List} of {@link String}s, one {@link String} per "line" of text.
+ *
+ * @author Ravi Nadahar - Initial contribution
+ */
+@NonNullByDefault
+public class TextOutputConsumer {
+
+    /**
+     * Consumes the specified {@link InputStream} by spawning a new thread and converts in into text using UTF-8. The
+     * result is returned as a {@link FutureTask}.
+     *
+     * @param inputStream the {@link InputStream} to consume.
+     * @return The {@link FutureTask} where the task can be cancelled and the results retrieved.
+     */
+    public FutureTask<List<String>> consume(InputStream inputStream) {
+        return consume(inputStream, null, null);
+    }
+
+    /**
+     * Consumes the specified {@link InputStream} by spawning a new thread and converts in into text using the
+     * specified {@link Charset}. The result is returned as a {@link FutureTask}.
+     *
+     * @param inputStream the {@link InputStream} to consume.
+     * @param charset the {@link Charset} to use for byte to character conversion.
+     *            will be generated.
+     * @return The {@link FutureTask} where the task can be cancelled and the results retrieved.
+     */
+    public FutureTask<List<String>> consume(InputStream inputStream, @Nullable Charset charset) {
+        return consume(inputStream, charset, null);
+    }
+
+    /**
+     * Consumes the specified {@link InputStream} by spawning a new thread and converts in into text using UTF-8. The
+     * result is returned as a {@link FutureTask}.
+     *
+     * @param inputStream the {@link InputStream} to consume.
+     * @param threadName the name of the worker thread that will do the consumption. If {@code null}, a thread name
+     *            will be generated.
+     * @return The {@link FutureTask} where the task can be cancelled and the results retrieved.
+     */
+    public FutureTask<List<String>> consume(InputStream inputStream, @Nullable String threadName) {
+        return consume(inputStream, null, threadName);
+    }
+
+    /**
+     * Consumes the specified {@link InputStream} by spawning a new thread and converts in into text using the
+     * specified {@link Charset}. The result is returned as a {@link FutureTask}.
+     *
+     * @param inputStream the {@link InputStream} to consume.
+     * @param charset the {@link Charset} to use for byte to character conversion.
+     * @param threadName the name of the worker thread that will do the consumption. If {@code null}, a thread name
+     *            will be generated.
+     * @return The {@link FutureTask} where the task can be cancelled and the results retrieved.
+     */
+    public FutureTask<List<String>> consume(final InputStream inputStream, @Nullable Charset charset,
+            @Nullable String threadName) {
+        FutureTask<List<String>> result = new FutureTask<List<String>>(() -> {
+            List<String> output = new ArrayList<>();
+            try (InputStreamReader isr = new InputStreamReader(inputStream,
+                    charset == null ? StandardCharsets.UTF_8 : charset); BufferedReader br = new BufferedReader(isr)) {
+                String line;
+                while ((line = br.readLine()) != null) {
+                    output.add(line);
+                }
+            }
+            return output;
+        });
+
+        Thread runner;
+        if (threadName == null || threadName.isBlank()) {
+            runner = new Thread(result);
+        } else {
+            runner = new Thread(result, threadName);
+        }
+        runner.start();
+        return result;
+    }
+}

bundles/org.openhab.binding.exec/src/main/resources/OH-INF/i18n/exec.properties

@@ -20,6 +20,8 @@ thing-type.config.exec.command.timeout.label = Timeout
 thing-type.config.exec.command.timeout.description = Time out, in seconds, the execution of the command will time out
 thing-type.config.exec.command.transform.label = Transform
 thing-type.config.exec.command.transform.description = The transformation to apply on the execution result, e.g. REGEX((.*)). You can chain transformations by listing each transformation on a separate line, or by separating them with the intersection character ∩.
+thing-type.config.exec.command.charset.label = Character Encoding
+thing-type.config.exec.command.charset.description = The character encoding to use for process output. Any string valid for Java's Charset.forName() can be used. If blank, UTF-8 will be used.
 
 # channel types
 
@@ -31,5 +33,9 @@ channel-type.exec.lastexecution.label = Last Execution
 channel-type.exec.lastexecution.description = Time/Date the command was last executed, in yyyy-MM-dd'T'HH:mm:ss.SSSZ format
 channel-type.exec.output.label = Output
 channel-type.exec.output.description = Output of the last execution of the command
+channel-type.exec.stdout.label = Standard Out
+channel-type.exec.stdout.description = The stdout output of the last execution of the command
+channel-type.exec.stderr.label = Standard Error
+channel-type.exec.stderr.description = The stderr output of the last execution of the command
 channel-type.exec.run.label = Running
 channel-type.exec.run.description = Send ON to execute the command and the current state tells whether it is running or not

bundles/org.openhab.binding.exec/src/main/resources/OH-INF/thing/thing-types.xml

@@ -10,12 +10,18 @@
 
 		<channels>
 			<channel id="output" typeId="output"/>
+			<channel id="stdout" typeId="stdout"/>
+			<channel id="stderr" typeId="stderr"/>
 			<channel id="input" typeId="input"/>
 			<channel id="exit" typeId="exit"/>
 			<channel id="run" typeId="run"/>
 			<channel id="lastexecution" typeId="lastexecution"/>
 		</channels>
 
+		<properties>
+			<property name="thingTypeVersion">1</property>
+		</properties>
+
 		<config-description>
 			<parameter name="command" type="text" required="true">
 				<label>Command</label>
@@ -43,6 +49,12 @@
 				<description>When true, the command will execute each time the state of the input channel changes</description>
 				<default>false</default>
 			</parameter>
+			<parameter name="charset" type="text" required="false">
+				<label>Character Encoding</label>
+				<description>The character encoding to use for process output. Any string valid for Java's Charset.forName() can be
+					used. If blank, UTF-8 will be used.</description>
+				<advanced>true</advanced>
+			</parameter>
 		</config-description>
 
 	</thing-type>
@@ -53,6 +65,18 @@
 		<description>Output of the last execution of the command</description>
 		<state readOnly="true"></state>
 	</channel-type>
+	<channel-type id="stdout">
+		<item-type>String</item-type>
+		<label>Standard Out</label>
+		<description>The stdout output of the last execution of the command</description>
+		<state readOnly="true"></state>
+	</channel-type>
+	<channel-type id="stderr">
+		<item-type>String</item-type>
+		<label>Standard Error</label>
+		<description>The stderr output of the last execution of the command</description>
+		<state readOnly="true"></state>
+	</channel-type>
 	<channel-type id="input">
 		<item-type>String</item-type>
 		<label>Input</label>

bundles/org.openhab.binding.exec/src/main/resources/OH-INF/update/update.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
+<update:update-descriptions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:update="https://openhab.org/schemas/update-description/v1.0.0"
+	xsi:schemaLocation="https://openhab.org/schemas/update-description/v1.0.0 https://openhab.org/schemas/update-description-1.0.0.xsd">
+
+	<thing-type uid="exec:command">
+		<instruction-set targetVersion="1">
+			<add-channel id="stdout">
+				<type>exec:stdout</type>
+				<label>Standard Out</label>
+			</add-channel>
+			<add-channel id="stderr">
+				<type>exec:stderr</type>
+				<label>Standard Error</label>
+			</add-channel>
+		</instruction-set>
+	</thing-type>
+</update:update-descriptions>