Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 9 additions & 21 deletions src/main/java/org/sentrysoftware/ipmi/client/IpmiClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

/**
Expand All @@ -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);
}
}

/**
Expand All @@ -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);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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;
}

}
4 changes: 3 additions & 1 deletion src/main/java/org/sentrysoftware/ipmi/client/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -73,7 +74,6 @@ public abstract class AbstractIpmiRunner<T> implements AutoCloseable {

protected IpmiClientConfiguration ipmiConfiguration;

private T result;

protected IpmiConnector connector;
protected ConnectionHandle handle;
Expand All @@ -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>
Expand All @@ -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()) {
Expand All @@ -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
*
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -122,7 +122,7 @@ public void doRun() throws Exception {

}

super.setResult(result);
return result;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();

Expand Down Expand Up @@ -119,7 +119,7 @@ public void doRun() throws Exception {

}

super.setResult(result);
return result;
}

/**
Expand Down
Loading