[denonmarantz] Run the Telnet socket in a dedicated thread (openhab#9511

)

* Run the Telnet socket in a dedicated thread, not from the shared thread pool.
Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
Signed-off-by: Joseph Hagberg <joseph@zoidberg.se>

bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java

@@ -29,9 +29,9 @@
 public class DenonMarantzConnectorFactory {
 
  public DenonMarantzConnector getConnector(DenonMarantzConfiguration config, DenonMarantzState state,
- ScheduledExecutorService scheduler, HttpClient httpClient) {
+ ScheduledExecutorService scheduler, HttpClient httpClient, String thingUID) {
  if (config.isTelnet()) {
- return new DenonMarantzTelnetConnector(config, state, scheduler);
+ return new DenonMarantzTelnetConnector(config, state, scheduler, thingUID);
  } else {
  return new DenonMarantzHttpConnector(config, state, scheduler, httpClient);
  }

bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClient.java → bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClientThread.java

@@ -31,9 +31,9 @@
  * @author Jeroen Idserda - Initial contribution (1.x Binding)
  * @author Jan-Willem Veldhuis - Refactored for 2.x
  */
-public class DenonMarantzTelnetClient implements Runnable {
+public class DenonMarantzTelnetClientThread extends Thread {
 
- private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClient.class);
+ private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClientThread.class);
 
  private static final Integer RECONNECT_DELAY = 60000; // 1 minute
 
@@ -43,8 +43,6 @@ public class DenonMarantzTelnetClient implements Runnable {
 
  private DenonMarantzTelnetListener listener;
 
- private boolean running = true;
-
  private boolean connected = false;
 
  private Socket socket;
@@ -53,15 +51,15 @@ public class DenonMarantzTelnetClient implements Runnable {
 
  private BufferedReader in;
 
- public DenonMarantzTelnetClient(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) {
+ public DenonMarantzTelnetClientThread(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) {
  logger.debug("Denon listener created");
  this.config = config;
  this.listener = listener;
  }
 
  @Override
  public void run() {
- while (running) {
+ while (!isInterrupted()) {
  if (!connected) {
  connectTelnetSocket();
  }
@@ -90,20 +88,21 @@ public void run() {
  connected = false;
  }
  } catch (IOException e) {
- if (!Thread.currentThread().isInterrupted()) {
+ if (!isInterrupted()) {
  // only log if we don't stop this on purpose causing a SocketClosed
  logger.debug("Error in telnet connection ", e);
  }
  connected = false;
  listener.telnetClientConnected(false);
  }
- } while (running && connected);
+ } while (!isInterrupted() && connected);
  }
+ disconnect();
+ logger.debug("Stopped client thread");
  }
 
  public void sendCommand(String command) {
  if (out != null) {
- logger.debug("Sending command {}", command);
  try {
  out.write(command + '\r');
  out.flush();
@@ -116,21 +115,21 @@ public void sendCommand(String command) {
  }
 
  public void shutdown() {
- this.running = false;
  disconnect();
  }
 
  private void connectTelnetSocket() {
  disconnect();
  int delay = 0;
 
- while (this.running && (socket == null || !socket.isConnected())) {
+ while (!isInterrupted() && (socket == null || !socket.isConnected())) {
  try {
  Thread.sleep(delay);
  logger.debug("Connecting to {}", config.getHost());
 
- // Use raw socket instead of TelnetClient here because TelnetClient sends an extra newline char
- // after each write which causes the connection to become unresponsive.
+ // Use raw socket instead of TelnetClient here because TelnetClient sends an
+ // extra newline char after each write which causes the connection to become
+ // unresponsive.
  socket = new Socket();
  socket.connect(new InetSocketAddress(config.getHost(), config.getTelnetPort()), TIMEOUT);
  socket.setKeepAlive(true);
@@ -141,6 +140,7 @@ private void connectTelnetSocket() {
 
  connected = true;
  listener.telnetClientConnected(true);
+ logger.debug("Denon telnet client connected to {}", config.getHost());
  } catch (IOException e) {
  logger.debug("Cannot connect to {}", config.getHost(), e);
  listener.telnetClientConnected(false);
@@ -149,8 +149,6 @@ private void connectTelnetSocket() {
  }
  delay = RECONNECT_DELAY;
  }
-
- logger.debug("Denon telnet client connected to {}", config.getHost());
  }
 
  public boolean isConnected() {

bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java

@@ -46,30 +46,32 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement
 
  private static final BigDecimal NINETYNINE = new BigDecimal("99");
 
- private DenonMarantzTelnetClient telnetClient;
+ private DenonMarantzTelnetClientThread telnetClientThread;
 
  private boolean displayNowplaying = false;
 
  protected boolean disposing = false;
 
  private Future<?> telnetStateRequest;
 
- private Future<?> telnetRunnable;
+ private String thingUID;
 
  public DenonMarantzTelnetConnector(DenonMarantzConfiguration config, DenonMarantzState state,
- ScheduledExecutorService scheduler) {
+ ScheduledExecutorService scheduler, String thingUID) {
  this.config = config;
  this.scheduler = scheduler;
  this.state = state;
+ this.thingUID = thingUID;
  }
 
  /**
  * Set up the connection to the receiver. Either using Telnet or by polling the HTTP API.
  */
  @Override
  public void connect() {
- telnetClient = new DenonMarantzTelnetClient(config, this);
- telnetRunnable = scheduler.submit(telnetClient);
+ telnetClientThread = new DenonMarantzTelnetClientThread(config, this);
+ telnetClientThread.setName("OH-binding-" + thingUID);
+ telnetClientThread.start();
  }
 
  @Override
@@ -98,9 +100,12 @@ public void dispose() {
  telnetStateRequest = null;
  }
 
- if (telnetClient != null && !telnetRunnable.isDone()) {
- telnetRunnable.cancel(true);
- telnetClient.shutdown();
+ if (telnetClientThread != null) {
+ telnetClientThread.interrupt();
+ // Invoke a shutdown after interrupting the thread to close the socket immediately,
+ // otherwise the client keeps running until a line was received from the telnet connection
+ telnetClientThread.shutdown();
+ telnetClientThread = null;
  }
  }
 
@@ -262,7 +267,7 @@ protected void internalSendCommand(String command) {
  logger.warn("Trying to send empty command");
  return;
  }
- telnetClient.sendCommand(command);
+ telnetClientThread.sendCommand(command);
  }
 
  /**

bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java

@@ -314,7 +314,8 @@ private void createConnection() {
  if (connector != null) {
  connector.dispose();
  }
- connector = connectorFactory.getConnector(config, denonMarantzState, scheduler, httpClient);
+ connector = connectorFactory.getConnector(config, denonMarantzState, scheduler, httpClient,
+ this.getThing().getUID().getAsString());
  connector.connect();
  }