Merge pull request #36 from sentrysoftware/feature/issue-35-high-cpu-…

…usage-due-to-infinite-loop-in-connection-classs-run-method

Issue #35: High CPU Usage Due to Infinite Loop in IPMI Core library `Connection.run()`

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);
+		}
 	}
 
 	/**

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,52 +57,158 @@ 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;
+	}
+
+	/**
+	 * 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;
+	}
+
 }

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();

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.

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);
-
 	}
 
 }

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;
 	}
 
 	/**

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;
 	}
 
 	/**