From a11b1451079f8b566dcdd72de5a00314c40387c2 Mon Sep 17 00:00:00 2001
From: Nassim Boutekedjiret <nassim@sentrysoftware.com>
Date: Thu, 6 Jun 2024 18:04:02 +0200
Subject: [PATCH 1/2] Issue #35: High CPU Usage Due to Infinite Loop in
 Connection.run()

* Implemented the ability to override the `pingPeriod`, allowing it to
be set to `0` to disable keep-alive messages.

* Added a delay (1 second) between retries to avoid tight looping and
reduce CPU usage.

* Included a check for thread interruption to allow graceful shutdown.

* Modified the runner code to ensure that closable instances are
properly closed.
---
 .../ipmi/client/IpmiClient.java               | 30 +++-----
 .../ipmi/client/IpmiClientConfiguration.java  | 29 +++++++-
 .../org/sentrysoftware/ipmi/client/Utils.java |  4 +-
 .../client/runner/AbstractIpmiRunner.java     | 38 +++-------
 .../client/runner/GetChassisStatusRunner.java |  7 +-
 .../ipmi/client/runner/GetFrusRunner.java     |  4 +-
 .../ipmi/client/runner/GetSensorsRunner.java  |  4 +-
 .../core/api/async/IpmiAsyncConnector.java    | 21 +++++-
 .../ipmi/core/api/sync/IpmiConnector.java     | 15 +++-
 .../ipmi/core/connection/Connection.java      | 70 ++++++++++++-------
 .../core/connection/ConnectionManager.java    | 23 ++++--
 src/site/markdown/index.md                    |  2 +
 12 files changed, 151 insertions(+), 96 deletions(-)

diff --git a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClient.java b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClient.java
index 7fc837d..2dcba00 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClient.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClient.java
@@ -24,7 +24,6 @@
 
 import static org.sentrysoftware.ipmi.client.Utils.execute;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
@@ -56,12 +55,9 @@ private IpmiClient() {
 	 */
 	public static GetChassisStatusResponseData getChassisStatus(final IpmiClientConfiguration ipmiConfiguration)
 			throws InterruptedException, ExecutionException, TimeoutException {
-		return execute(() -> {
-			try (GetChassisStatusRunner runner = new GetChassisStatusRunner(ipmiConfiguration)) {
-				runner.doRun();
-				return runner.getResult();
-			}
-		}, ipmiConfiguration.getTimeout() * 1000);
+		try (GetChassisStatusRunner runner = new GetChassisStatusRunner(ipmiConfiguration)){
+			return execute(runner, ipmiConfiguration.getTimeout() * 1000);
+		}
 	}
 
 	/**
@@ -76,13 +72,9 @@ public static GetChassisStatusResponseData getChassisStatus(final IpmiClientConf
 	 */
 	public static List<Sensor> getSensors(final IpmiClientConfiguration ipmiConfiguration)
 			throws InterruptedException, ExecutionException, TimeoutException {
-		return execute(() -> {
-			try (GetSensorsRunner runner = new GetSensorsRunner(ipmiConfiguration)) {
-				runner.doRun();
-				List<Sensor> result = runner.getResult();
-				return result != null ? result : new ArrayList<>();
-			}
-		}, ipmiConfiguration.getTimeout() * 1000);
+		try (GetSensorsRunner runner = new GetSensorsRunner(ipmiConfiguration)) {
+			return execute(runner, ipmiConfiguration.getTimeout() * 1000);
+		}
 	}
 
 	/**
@@ -97,13 +89,9 @@ public static List<Sensor> getSensors(final IpmiClientConfiguration ipmiConfigur
 	 */
 	public static List<Fru> getFrus(final IpmiClientConfiguration ipmiConfiguration)
 			throws InterruptedException, ExecutionException, TimeoutException {
-		return execute(() -> {
-			try (GetFrusRunner runner = new GetFrusRunner(ipmiConfiguration)) {
-				runner.doRun();
-				List<Fru> result = runner.getResult();
-				return result != null ? result : new ArrayList<>();
-			}
-		}, ipmiConfiguration.getTimeout() * 1000);
+		try (GetFrusRunner runner = new GetFrusRunner(ipmiConfiguration)) {
+			return execute(runner, ipmiConfiguration.getTimeout() * 1000);
+		}
 	}
 
 	/**
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
index 82d1373..ddadbe1 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
@@ -24,7 +24,7 @@
 
 /**
  * IPMI configuration including the required credentials that need to be used to establish the
- * communication with the IPMI interface
+ * communication with the IPMI interface.
  *
  */
 public class IpmiClientConfiguration {
@@ -35,6 +35,7 @@ public class IpmiClientConfiguration {
 	private byte[] bmcKey;
 	private boolean skipAuth;
 	private long timeout;
+	private long pingPeriod = -1;
 
 	/**
 	 * Instantiates a new {@link IpmiClientConfiguration} in order to query the IPMI host.
@@ -56,6 +57,24 @@ public IpmiClientConfiguration(String hostname, String username, char[] password
 		this.timeout = timeout;
 	}
 
+	/**
+	 * Instantiates a new {@link IpmiClientConfiguration} in order to query the IPMI host.
+	 * 
+	 * @param hostname   IP Address or host name of the remote IPMI host.
+	 * @param username   Name used to establish the connection with the host via the IPMI protocol.
+	 * @param password   Password used to establish the connection with the host via the IPMI protocol.
+	 * @param bmcKey     The key that should be provided if the two-key authentication is enabled, null otherwise.
+	 * @param skipAuth   Whether the client should skip authentication
+	 * @param timeout    Timeout used for each IPMI request.
+	 * @param pingPeriod The period in milliseconds used to send the keep alive messages.<br>
+	 *                   Set pingPeriod to 0 to turn off keep-alive messages sent to the remote host.
+	 */
+	public IpmiClientConfiguration(String hostname, String username, char[] password,
+			byte[] bmcKey, boolean skipAuth, long timeout, long pingPeriod) {
+		this(hostname, username, password, bmcKey, skipAuth, timeout);
+		this.pingPeriod = pingPeriod;
+	}
+
 	public String getHostname() {
 		return hostname;
 	}
@@ -104,4 +123,12 @@ public void setTimeout(long timeout) {
 		this.timeout = timeout;
 	}
 
+	public long getPingPeriod() {
+		return pingPeriod;
+	}
+
+	public void setPingPeriod(long pingPeriod) {
+		this.pingPeriod = pingPeriod;
+	}
+
 }
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/Utils.java b/src/main/java/org/sentrysoftware/ipmi/client/Utils.java
index 774b30a..b8c1a66 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/Utils.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/Utils.java
@@ -30,6 +30,8 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
+import org.sentrysoftware.ipmi.client.runner.AbstractIpmiRunner;
+
 public class Utils {
 
 	private Utils() {}
@@ -81,7 +83,7 @@ public static String getValueOrEmpty(String value) {
 	 * @throws ExecutionException
 	 * @throws TimeoutException
 	 */
-	public static <T> T execute(Callable<T> callable, long timeout)
+	public static <T> T execute(final AbstractIpmiRunner<T> callable, long timeout)
 			throws InterruptedException, ExecutionException, TimeoutException {
 
 		final ExecutorService executorService = Executors.newSingleThreadExecutor();
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/runner/AbstractIpmiRunner.java b/src/main/java/org/sentrysoftware/ipmi/client/runner/AbstractIpmiRunner.java
index 97155d9..1d8095c 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/runner/AbstractIpmiRunner.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/runner/AbstractIpmiRunner.java
@@ -24,6 +24,7 @@
 
 import java.net.InetAddress;
 import java.util.List;
+import java.util.concurrent.Callable;
 
 import org.sentrysoftware.ipmi.client.IpmiClientConfiguration;
 import org.sentrysoftware.ipmi.core.api.async.ConnectionHandle;
@@ -45,7 +46,7 @@
  * 
  * @param <T> Represent the data type managed by the runner 
  */
-public abstract class AbstractIpmiRunner<T> implements AutoCloseable {
+public abstract class AbstractIpmiRunner<T> implements AutoCloseable, Callable<T> {
 
 	private static final int DEFAULT_LOCAL_UDP_PORT = 0;
 
@@ -73,7 +74,6 @@ public abstract class AbstractIpmiRunner<T> implements AutoCloseable {
 
 	protected IpmiClientConfiguration ipmiConfiguration;
 
-	private T result;
 
 	protected IpmiConnector connector;
 	protected ConnectionHandle handle;
@@ -84,13 +84,6 @@ protected AbstractIpmiRunner(IpmiClientConfiguration ipmiConfiguration) {
 		this.ipmiConfiguration = ipmiConfiguration;
 	}
 
-	/**
-	 * Proceed with the IPMI request
-	 * 
-	 * @throws Exception If the sensor or fru retrieval fails
-	 */
-	public abstract void doRun() throws Exception;
-
 	/**
 	 * Create the {@link IpmiConnector} instance, perform the authentication if required then start the session. <br>
 	 * This method will instantiate the internal fields: <em></em>
@@ -101,7 +94,7 @@ protected void startSession() throws Exception {
 		// Create the connector, specify port that will be used to communicate
 		// with the remote host. The UDP layer starts listening at this port, so
 		// no 2 connectors can work at the same time on the same port.
-		connector = new IpmiConnector(DEFAULT_LOCAL_UDP_PORT);
+		connector = new IpmiConnector(DEFAULT_LOCAL_UDP_PORT, ipmiConfiguration.getPingPeriod());
 
 		// Should we perform the authentication
 		if (!ipmiConfiguration.isSkipAuth()) {
@@ -118,23 +111,6 @@ protected void startSession() throws Exception {
 				String.valueOf(ipmiConfiguration.getPassword()), ipmiConfiguration.getBmcKey());
 	}
 
-	/**
-	 * 
-	 * @return The actual result set by the concrete runner
-	 */
-	public T getResult() {
-		return result;
-	}
-
-	/**
-	 * Set the final result
-	 * 
-	 * @param result The concrete runner result
-	 */
-	public void setResult(T result) {
-		this.result = result;
-	}
-
 	/**
 	 * Authenticate IPMI
 	 * 
@@ -185,10 +161,14 @@ protected CipherSuite getAvailableCipherSuite() throws Exception {
 	}
 
 	@Override
-	public void close() throws Exception {
+	public void close() {
 		if (handle != null) {
 			// Close the session
-			connector.closeSession(handle);
+			try {
+				connector.closeSession(handle);
+			} catch (Exception e) {
+				// Ignore
+			}
 		}
 
 		// Close connection manager and release the listener port.
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetChassisStatusRunner.java b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetChassisStatusRunner.java
index c723a3c..90d1e3c 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetChassisStatusRunner.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetChassisStatusRunner.java
@@ -38,16 +38,13 @@ public GetChassisStatusRunner(IpmiClientConfiguration ipmiConfiguration) {
 	}
 
 	@Override
-	public void doRun() throws Exception {
-
+	public GetChassisStatusResponseData call() throws Exception {
 		super.startSession();
 
 		// Send the UDP message and read the response
-		GetChassisStatusResponseData rd = (GetChassisStatusResponseData) connector.sendMessage(handle,
+		return (GetChassisStatusResponseData) connector.sendMessage(handle,
 				new GetChassisStatus(IpmiVersion.V20, handle.getCipherSuite(), AuthenticationType.RMCPPlus));
 
-		super.setResult(rd);
-
 	}
 
 }
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetFrusRunner.java b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetFrusRunner.java
index 1ce311f..b727615 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetFrusRunner.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetFrusRunner.java
@@ -71,7 +71,7 @@ public GetFrusRunner(IpmiClientConfiguration ipmiConfiguration) {
 	}
 
 	@Override
-	public void doRun() throws Exception {
+	public List<Fru> call() throws Exception {
 		final List<Fru> result = new ArrayList<>();
 
 		super.startSession();
@@ -122,7 +122,7 @@ public void doRun() throws Exception {
 
 		}
 
-		super.setResult(result);
+		return result;
 	}
 
 	/**
diff --git a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetSensorsRunner.java b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetSensorsRunner.java
index 13ae1a7..980fdc7 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/runner/GetSensorsRunner.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/runner/GetSensorsRunner.java
@@ -57,7 +57,7 @@ public GetSensorsRunner(IpmiClientConfiguration ipmiConfiguration) {
 	}
 
 	@Override
-	public void doRun() throws Exception {
+	public List<Sensor> call() throws Exception {
 
 		final List<Sensor> result = new ArrayList<>();
 
@@ -119,7 +119,7 @@ public void doRun() throws Exception {
 
 		}
 
-		super.setResult(result);
+		return result;
 	}
 
 	/**
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java b/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
index 8dbd9d9..e31f9b0 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
@@ -131,7 +131,26 @@ public IpmiAsyncConnector(int port, InetAddress address) throws IOException {
         loadProperties();
     }
 
-    private void loadProperties() {
+	/**
+	 * Starts {@link IpmiAsyncConnector} and initiates the {@link ConnectionManager}
+	 * at the given port and ping period.
+	 * 
+	 * @param port       the port that will be used by {@link IpmiAsyncConnector} to
+	 *                   communicate with the remote hosts.
+	 * @param pingPeriod the period between sending keep-alive messages to the
+	 *                   remote host.
+	 * @throws IOException When ConnectionManager cannot be created due to an IO
+	 *                     error.
+	 */
+	public IpmiAsyncConnector(int port, long pingPeriod) throws IOException {
+		responseListeners = new ArrayList<>();
+		inboundMessageListeners = new ArrayList<>();
+		connectionManager = new ConnectionManager(port, pingPeriod);
+		sessionManager = new SessionManager();
+		loadProperties();
+	}
+
+	private void loadProperties() {
         retries = Integer.parseInt(PropertiesManager.getInstance().getProperty("retries"));
     }
 
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java b/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
index d19625d..d8b8709 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
@@ -109,7 +109,20 @@ public IpmiConnector(int port, InetAddress address) throws IOException {
         loadProperties();
     }
 
-    private void loadProperties() {
+    /**
+     * Starts {@link IpmiConnector} and initiates the {@link ConnectionManager} at the given port. Wildcard IP address
+     * will be used.
+     * <br>
+     * @param port       the port that will be used by {@link IpmiAsyncConnector} to communicate with the remote hosts.
+     * @param pingPeriod
+     * @throws IOException
+     */
+	public IpmiConnector(int port, long pingPeriod) throws IOException {
+		asyncConnector = new IpmiAsyncConnector(port, pingPeriod);
+		loadProperties();
+	}
+
+	private void loadProperties() {
         PropertiesManager manager = PropertiesManager.getInstance();
         retries = Integer.parseInt(manager.getProperty("retries"));
         idleTime = Integer.parseInt(manager.getProperty("idleTime"));
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/connection/Connection.java b/src/main/java/org/sentrysoftware/ipmi/core/connection/Connection.java
index 3c31e77..7c8fbb5 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/connection/Connection.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/connection/Connection.java
@@ -178,7 +178,7 @@ public void unregisterListener(ConnectionListener listener) {
      *             - when properties file was not found
      * @see #disconnect()
      */
-    public void connect(InetAddress address, int port, int pingPeriod)
+    public void connect(InetAddress address, int port, long pingPeriod)
             throws IOException {
         connect(address, port, pingPeriod, false);
     }
@@ -197,15 +197,20 @@ public void connect(InetAddress address, int port, int pingPeriod)
      * - when properties file was not found
      * @see #disconnect()
      */
-    public void connect(InetAddress address, int port, int pingPeriod, boolean skipCiphers) throws IOException {
+    public void connect(InetAddress address, int port, long pingPeriod, boolean skipCiphers) throws IOException {
         MessageHandler ipmiMessageHandler = new IpmiMessageHandler(this, timeout);
         messageHandlers.put(PayloadType.Ipmi, ipmiMessageHandler);
 
         MessageHandler solMessageHandler = new SolMessageHandler(this, timeout);
         messageHandlers.put(PayloadType.Sol, solMessageHandler);
 
-        timer = new Timer();
-        timer.schedule(this, pingPeriod, pingPeriod);
+		// If the pingPeriod greater than 0, start the timer otherwise don't start it
+		// means that the connection won't be kept alive by sending no-op messages
+		if (pingPeriod > 0) {
+			timer = new Timer();
+			timer.schedule(this, pingPeriod, pingPeriod);
+		}
+
         stateMachine.register(this);
         if (skipCiphers) {
             stateMachine.start(address, port);
@@ -222,7 +227,10 @@ public void connect(InetAddress address, int port, int pingPeriod, boolean skipC
      * @see #connect(InetAddress, int, int)
      */
     public void disconnect() {
-        timer.cancel();
+		if (timer != null) {
+			timer.cancel();
+		}
+
         stateMachine.stop();
 
         for (MessageHandler messageHandler : messageHandlers.values()) {
@@ -601,28 +609,36 @@ public void notify(StateMachineAction action) {
         }
     }
 
-    /**
-     * {@link TimerTask} runner - periodically sends no-op messages to keep the
-     * session up
-     */
-    @Override
-    public void run() {
-        int result = -1;
-        do {
-            try {
-                if (!(stateMachine.getCurrent() instanceof SessionValid)) {
-                    break;
-                }
-                result = sendMessage(new org.sentrysoftware.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilities(
-                        IpmiVersion.V20, IpmiVersion.V20,
-                        ((SessionValid) stateMachine.getCurrent())
-                                .getCipherSuite(), PrivilegeLevel.Callback,
-                        TypeConverter.intToByte(0xe)), false);
-            } catch (Exception e) {
-                logger.error(e.getMessage(), e);
-            }
-        } while (result <= 0);
-    }
+	/**
+	 * {@link TimerTask} runner - periodically sends no-op messages to keep the
+	 * session up
+	 */
+	@Override
+	public void run() {
+		int result = -1;
+		while (
+			!Thread.currentThread().isInterrupted()
+			&& result <= 0
+			&& stateMachine.getCurrent() instanceof SessionValid
+		) {
+			try {
+
+				result = sendMessage(
+					new org.sentrysoftware.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilities(
+						IpmiVersion.V20, IpmiVersion.V20,
+						((SessionValid) stateMachine.getCurrent()).getCipherSuite(), PrivilegeLevel.Callback,
+						TypeConverter.intToByte(0xe)
+					),
+					false
+				);
+
+				Thread.sleep(1000);
+
+			} catch (Exception e) {
+				logger.error(e.getMessage(), e);
+			}
+		}
+	}
 
     public InetAddress getRemoteMachineAddress() {
         return stateMachine.getRemoteMachineAddress();
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java b/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
index c0f3689..0b55043 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
@@ -49,15 +49,27 @@ public class ConnectionManager {
     /**
      * Frequency of the no-op commands that will be sent to keep up the session
      */
-    private static int pingPeriod = -1;
+    private long pingPeriod = -1;
+
+	/**
+	 * Initiates the connection manager. Wildcard IP address will be used.
+	 *
+	 * @param port       the port at which {@link UdpListener} will work
+	 * @param pingPeriod frequency of the no-op commands that will be sent to keep
+	 *                   up the session
+	 * @throws IOException If UdpMessenger encountered an error
+	 */
+	public ConnectionManager(int port, long pingPeriod) throws IOException {
+		this(port);
+		this.pingPeriod = pingPeriod;
+	}
 
     /**
      * Initiates the connection manager. Wildcard IP address will be used.
      *
      * @param port
      *            - the port at which {@link UdpListener} will work
-     * @throws IOException
-     *             when properties file was not found
+     * @throws IOException If UdpMessenger encountered an error
      */
     public ConnectionManager(int port) throws IOException {
         messenger = new UdpMessenger(port);
@@ -71,8 +83,7 @@ public ConnectionManager(int port) throws IOException {
      *            - the port at which {@link UdpListener} will work
      * @param address
      *            - the IP interface {@link UdpListener} will bind to
-     * @throws IOException
-     *             when properties file was not found
+     * @throws IOException If UdpMessenger encountered an error
      */
     public ConnectionManager(int port, InetAddress address) throws IOException {
         messenger = new UdpMessenger(port, address);
@@ -94,7 +105,7 @@ private void initialize() {
         connections = new ArrayList<Connection>();
         reservedTags = new ArrayList<Integer>();
         if (pingPeriod == -1) {
-            pingPeriod = Integer.parseInt(PropertiesManager.getInstance().getProperty("pingPeriod"));
+            pingPeriod = Long.parseLong(PropertiesManager.getInstance().getProperty("pingPeriod"));
         }
     }
 
diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md
index 1676086..879e0b7 100644
--- a/src/site/markdown/index.md
+++ b/src/site/markdown/index.md
@@ -29,6 +29,8 @@ Invoke the IPMI Client:
 		final boolean noAuth = false;
 		final byte[] bmcKey = null;
 		final long timeout = 120;
+		// Set pingPeriod to 0 to turn off keep-alive messages sent to the remote host.
+		final pingPeriod = 30000;
 
 		// Instantiates a new IPMI client configuration using the credentials above
 		final IpmiClientConfiguration ipmiClientConfiguration = new IpmiClientConfiguration(

From 06ed1ac5d751382efb76daf31e39e65cbfcbd50f Mon Sep 17 00:00:00 2001
From: Nassim Boutekedjiret <nassim@sentrysoftware.com>
Date: Fri, 7 Jun 2024 11:57:26 +0200
Subject: [PATCH 2/2] Issue #35: High CPU Usage Due to Infinite Loop in
 Connection.run()

* Fixed javadoc.
---
 .../ipmi/client/IpmiClientConfiguration.java  | 80 +++++++++++++++++++
 .../core/api/async/IpmiAsyncConnector.java    |  7 +-
 .../ipmi/core/api/sync/IpmiConnector.java     | 30 ++++---
 .../core/connection/ConnectionManager.java    |  2 +-
 4 files changed, 104 insertions(+), 15 deletions(-)

diff --git a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
index ddadbe1..8ef2c94 100644
--- a/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
+++ b/src/main/java/org/sentrysoftware/ipmi/client/IpmiClientConfiguration.java
@@ -75,58 +75,138 @@ public IpmiClientConfiguration(String hostname, String username, char[] password
 		this.pingPeriod = pingPeriod;
 	}
 
+	/**
+	 * Returns the IP Address or host name of the remote IPMI host.
+	 * 
+	 * @return IP Address or host name of the remote IPMI host.
+	 */
 	public String getHostname() {
 		return hostname;
 	}
 
+	/**
+	 * Sets the IP Address or host name of the remote IPMI host.
+	 * 
+	 * @param hostname IP Address or host name of the remote IPMI host.
+	 */
 	public void setHostname(String hostname) {
 		this.hostname = hostname;
 	}
 
+	/**
+	 * Returns the name used to establish the connection with the host via the IPMI
+	 * protocol.
+	 * 
+	 * @return Name used to establish the connection with the host via the IPMI protocol.
+	 */
 	public String getUsername() {
 		return username;
 	}
 
+	/**
+	 * Sets the name used to establish the connection with the host via the IPMI
+	 * protocol.
+	 * 
+	 * @param username Name used to establish the connection with the host via the
+	 *                 IPMI protocol.
+	 */
 	public void setUsername(String username) {
 		this.username = username;
 	}
 
+	/**
+	 * Returns the password used to establish the connection with the host via the
+	 * IPMI protocol.
+	 * 
+	 * @return Password used to establish the connection with the host via the IPMI protocol.
+	 */
 	public char[] getPassword() {
 		return password;
 	}
 
+	/**
+	 * Sets the password used to establish the connection with the host via the IPMI
+	 * protocol.
+	 * 
+	 * @param password Password used to establish the connection with the host via the IPMI protocol.
+	 */
 	public void setPassword(char[] password) {
 		this.password = password;
 	}
 
+	/**
+	 * Returns the key that should be provided if the two-key authentication is
+	 * enabled, null otherwise.
+	 * 
+	 * @return The key that should be provided if the two-key authentication is
+	 *         enabled, null otherwise.
+	 */
 	public byte[] getBmcKey() {
 		return bmcKey;
 	}
 
+	/**
+	 * Sets the key that should be provided if the two-key authentication is
+	 * enabled, null otherwise.
+	 * 
+	 * @param bmcKey The key that should be provided if the two-key authentication
+	 *               is enabled, null otherwise.
+	 */
 	public void setBmcKey(byte[] bmcKey) {
 		this.bmcKey = bmcKey;
 	}
 
+	/**
+	 * Returns whether the client should skip authentication.
+	 * 
+	 * @return Whether the client should skip authentication.
+	 */
 	public boolean isSkipAuth() {
 		return skipAuth;
 	}
 
+	/**
+	 * Sets whether the client should skip authentication.
+	 * 
+	 * @param skipAuth Whether the client should skip authentication.
+	 */
 	public void setSkipAuth(boolean skipAuth) {
 		this.skipAuth = skipAuth;
 	}
 
+	/**
+	 * Returns the timeout used for each IPMI request.
+	 * 
+	 * @return The timeout used for each IPMI request.
+	 */
 	public long getTimeout() {
 		return timeout;
 	}
 
+	/**
+	 * Sets the timeout used for each IPMI request.
+	 * 
+	 * @param timeout The timeout used for each IPMI request.
+	 */
 	public void setTimeout(long timeout) {
 		this.timeout = timeout;
 	}
 
+	/**
+	 * Returns the period in milliseconds used to send the keep alive messages.
+	 * 
+	 * @return The period in milliseconds used to send the keep alive messages.
+	 */
 	public long getPingPeriod() {
 		return pingPeriod;
 	}
 
+	/**
+	 * Sets the period in milliseconds used to send the keep alive messages.<br>
+	 * Set pingPeriod to 0 to turn off keep-alive messages sent to the remote host.
+	 * 
+	 * @param pingPeriod The period in milliseconds used to send the keep alive messages.
+	 */
 	public void setPingPeriod(long pingPeriod) {
 		this.pingPeriod = pingPeriod;
 	}
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java b/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
index e31f9b0..7aff732 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/api/async/IpmiAsyncConnector.java
@@ -150,9 +150,12 @@ public IpmiAsyncConnector(int port, long pingPeriod) throws IOException {
 		loadProperties();
 	}
 
+	/**
+	 * Loads properties from the properties file.
+	 */
 	private void loadProperties() {
-        retries = Integer.parseInt(PropertiesManager.getInstance().getProperty("retries"));
-    }
+		retries = Integer.parseInt(PropertiesManager.getInstance().getProperty("retries"));
+	}
 
     /**
      * Creates connection to the remote host.
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java b/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
index d8b8709..3100ab0 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/api/sync/IpmiConnector.java
@@ -109,24 +109,30 @@ public IpmiConnector(int port, InetAddress address) throws IOException {
         loadProperties();
     }
 
-    /**
-     * Starts {@link IpmiConnector} and initiates the {@link ConnectionManager} at the given port. Wildcard IP address
-     * will be used.
-     * <br>
-     * @param port       the port that will be used by {@link IpmiAsyncConnector} to communicate with the remote hosts.
-     * @param pingPeriod
-     * @throws IOException
-     */
+	/**
+	 * Starts {@link IpmiConnector} and initiates the {@link ConnectionManager} at
+	 * the given port. Wildcard IP address will be used.
+	 * 
+	 * @param port       the port that will be used by {@link IpmiAsyncConnector} to
+	 *                   communicate with the remote hosts.
+	 * @param pingPeriod the period in milliseconds used to send the keep alive
+	 *                   messages.<br>
+	 *                   0 If keep-alive messages should be disabled.
+	 * @throws IOException When IpmiAsyncConnector cannot be created.
+	 */
 	public IpmiConnector(int port, long pingPeriod) throws IOException {
 		asyncConnector = new IpmiAsyncConnector(port, pingPeriod);
 		loadProperties();
 	}
 
+	/**
+	 * Loads properties from the properties file.
+	 */
 	private void loadProperties() {
-        PropertiesManager manager = PropertiesManager.getInstance();
-        retries = Integer.parseInt(manager.getProperty("retries"));
-        idleTime = Integer.parseInt(manager.getProperty("idleTime"));
-    }
+		PropertiesManager manager = PropertiesManager.getInstance();
+		retries = Integer.parseInt(manager.getProperty("retries"));
+		idleTime = Integer.parseInt(manager.getProperty("idleTime"));
+	}
 
     /**
      * Creates connection to the remote host on default IPMI port.
diff --git a/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java b/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
index 0b55043..1b40eff 100644
--- a/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
+++ b/src/main/java/org/sentrysoftware/ipmi/core/connection/ConnectionManager.java
@@ -56,7 +56,7 @@ public class ConnectionManager {
 	 *
 	 * @param port       the port at which {@link UdpListener} will work
 	 * @param pingPeriod frequency of the no-op commands that will be sent to keep
-	 *                   up the session
+	 *                   up the session. 0 to disable ping requests.
 	 * @throws IOException If UdpMessenger encountered an error
 	 */
 	public ConnectionManager(int port, long pingPeriod) throws IOException {