Skip to content

Commit c86bd4e

Browse files
authored
Merge pull request #148 from MetricsHub/fix/connection-lifecycle
Fix the connection and manager lifecycle (#126, #94, #95, #98)
2 parents 1abf019 + 7f032dc commit c86bd4e

19 files changed

Lines changed: 651 additions & 123 deletions

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ The BMC must have IPMI over LAN enabled and an account with the User privilege:
2020

2121
## Upgrading
2222

23-
Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. The user name and password are now encoded in UTF-8 whatever the platform charset, and the BMC key is used as raw bytes; as a result, `AuthenticationAlgorithm.getKeyExchangeAuthenticationCode()` and `checkKeyExchangeAuthenticationCode()` take the key and password as `byte[]` instead of `String`. `UdpMessenger.getSentPackets()` is removed, and the `CONST1`/`CONST2` constants of `IntegrityAlgorithm` and `ConfidentialityAesCbc128` are now `private`.
23+
Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. The user name and password are now encoded in UTF-8 whatever the platform charset, and the BMC key is used as raw bytes; as a result, `AuthenticationAlgorithm.getKeyExchangeAuthenticationCode()` and `checkKeyExchangeAuthenticationCode()` take the key and password as `byte[]` instead of `String`. `UdpMessenger.getSentPackets()` is removed, and the `CONST1`/`CONST2` constants of `IntegrityAlgorithm` and `ConfidentialityAesCbc128` are now `private`. `IpmiConnector.closeConnection()` releases the connection, whose handle then throws `IllegalStateException`; the keep-alive is actually sent with the default configuration, as a `Connection.KeepAlive` request whose reply and timeout are not reported to the listeners; and `Constants.TIMEOUT`, which nothing reads, is deprecated.
2424

2525
## Build instructions
2626

‎src/main/java/org/metricshub/ipmi/core/common/Constants.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ public final class Constants {
3232
*/
3333
public static final int IPMI_PORT = 0x26F;
3434

35+
/**
36+
* Unused by the library.
37+
*
38+
* @deprecated nothing reads it; it will be removed in the next major version.
39+
*/
40+
@Deprecated
3541
public static final int TIMEOUT = 500;
3642

3743
private Constants() {}

‎src/main/java/org/metricshub/ipmi/core/common/PropertiesManager.java‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,38 +26,50 @@
2626

2727
import java.io.IOException;
2828
import java.io.InputStream;
29-
import java.util.HashMap;
3029
import java.util.Map;
3130
import java.util.Properties;
31+
import java.util.concurrent.ConcurrentHashMap;
3232

3333
import org.slf4j.Logger;
3434
import org.slf4j.LoggerFactory;
3535

3636
public final class PropertiesManager {
3737

38+
private static final Object INSTANCE_LOCK = new Object();
3839
private static PropertiesManager instance;
3940

4041
private Map<String, String> properties;
4142

4243
private Logger logger = LoggerFactory.getLogger(PropertiesManager.class);
4344

4445
private PropertiesManager() {
45-
properties = new HashMap<String, String>();
46+
properties = new ConcurrentHashMap<String, String>();
4647

4748
loadProperties("/connection.properties");
4849
loadProperties("/vxipmi.properties");
4950
}
5051

5152
@SuppressFBWarnings(value = "MS_EXPOSE_REP", justification = "Singleton: handing out the shared instance is the point")
5253
public static PropertiesManager getInstance() {
53-
if (instance == null) {
54-
instance = new PropertiesManager();
54+
synchronized (INSTANCE_LOCK) {
55+
if (instance == null) {
56+
instance = new PropertiesManager();
57+
}
58+
return instance;
5559
}
56-
return instance;
5760
}
5861

59-
private void loadProperties(String name) {
62+
/**
63+
* Adds the properties of a classpath resource; a missing resource is logged and skipped.
64+
*
65+
* @param name the resource name, such as {@code /connection.properties}
66+
*/
67+
void loadProperties(String name) {
6068
try (InputStream stream = getClass().getResourceAsStream(name)) {
69+
if (stream == null) {
70+
logger.error("Properties resource {} not found", name);
71+
return;
72+
}
6173
Properties props = new Properties();
6274
props.load(stream);
6375

@@ -71,7 +83,7 @@ private void loadProperties(String name) {
7183
}
7284

7385
public String getProperty(String key) {
74-
logger.info("Getting " + key + ": " + properties.get(key));
86+
logger.debug("Getting {}: {}", key, properties.get(key));
7587
return properties.get(key);
7688
}
7789

‎src/main/java/org/metricshub/ipmi/core/connection/Connection.java‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.metricshub.ipmi.core.coding.commands.IpmiVersion;
2727
import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel;
2828
import org.metricshub.ipmi.core.coding.commands.ResponseData;
29+
import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilities;
2930
import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilitiesResponseData;
3031
import org.metricshub.ipmi.core.coding.commands.session.GetChannelCipherSuitesResponseData;
3132
import org.metricshub.ipmi.core.coding.commands.session.OpenSessionResponseData;
@@ -61,6 +62,7 @@
6162
import org.metricshub.ipmi.core.sm.states.Authcap;
6263
import org.metricshub.ipmi.core.sm.states.Ciphers;
6364
import org.metricshub.ipmi.core.sm.states.SessionValid;
65+
import org.metricshub.ipmi.core.sm.states.State;
6466
import org.metricshub.ipmi.core.sm.states.Uninitialized;
6567
import org.metricshub.ipmi.core.transport.Messenger;
6668

@@ -666,31 +668,35 @@ public void notify(StateMachineAction action) {
666668
}
667669

668670
/**
669-
* {@link TimerTask} runner - periodically sends no-op messages to keep the
670-
* session up
671+
* The keep-alive request: queued like any request, so that its tag stays reserved until its reply arrives or it
672+
* times out, but owned by nobody: its reply is discarded and its timeout is not reported to the listeners.
673+
*/
674+
public static final class KeepAlive extends GetChannelAuthenticationCapabilities {
675+
/**
676+
* Creates the keep-alive request of a session.
677+
*
678+
* @param cipherSuite the {@link CipherSuite} of the session
679+
*/
680+
public KeepAlive(CipherSuite cipherSuite) {
681+
super(IpmiVersion.V20, IpmiVersion.V20, cipherSuite, PrivilegeLevel.Callback, TypeConverter.intToByte(0xe));
682+
}
683+
}
684+
685+
/**
686+
* {@link TimerTask} runner - periodically sends a no-op message to keep the session up. The message is a
687+
* {@link KeepAlive}: its reply is discarded, while a reply to the same command sent by the application is
688+
* delivered to the application. When the message queue is full, the keep-alive of this period is skipped.
671689
*/
672690
@Override
673691
public void run() {
674-
int result = -1;
675-
while (!Thread.currentThread().isInterrupted()
676-
&& result <= 0
677-
&& stateMachine.getCurrent() instanceof SessionValid) {
678-
try {
679-
680-
result = sendMessage(
681-
new org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilities(
682-
IpmiVersion.V20,
683-
IpmiVersion.V20,
684-
((SessionValid) stateMachine.getCurrent()).getCipherSuite(),
685-
PrivilegeLevel.Callback,
686-
TypeConverter.intToByte(0xe)),
687-
false);
688-
689-
Thread.sleep(1000);
690-
691-
} catch (Exception e) {
692-
LOGGER.error(e.getMessage(), e);
693-
}
692+
State current = stateMachine.getCurrent();
693+
if (!stateMachine.isActive() || !(current instanceof SessionValid)) {
694+
return;
695+
}
696+
try {
697+
sendMessage(new KeepAlive(((SessionValid) current).getCipherSuite()), false);
698+
} catch (Exception e) {
699+
LOGGER.error("Keep-alive failed: " + e.getMessage(), e);
694700
}
695701
}
696702

0 commit comments

Comments
 (0)