Skip to content

Commit edba412

Browse files
committed
GenericJMX plugin: memory leak, indent and compile warn fixes
- call close() on JMXConnector if JMX connection fails; fixes memory leak - fix indentation errors - fix "unchecked" compile warnings - some refactoring
1 parent 306810f commit edba412

File tree

1 file changed

+64
-45
lines changed

1 file changed

+64
-45
lines changed

bindings/java/org/collectd/java/GenericJMXConfConnection.java

+64-45
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ class GenericJMXConfConnection
5252
private String _host = null;
5353
private String _instance_prefix = null;
5454
private String _service_url = null;
55-
private MBeanServerConnection _jmx_connection = null;
55+
private JMXConnector _jmx_connector = null;
56+
private MBeanServerConnection _mbean_connection = null;
5657
private List<GenericJMXConfMBean> _mbeans = null;
5758

5859
/*
@@ -92,55 +93,72 @@ private String getHost () /* {{{ */
9293
return Collectd.getHostname();
9394
} /* }}} String getHost */
9495

95-
private void connect () /* {{{ */
96-
{
97-
JMXServiceURL service_url;
98-
JMXConnector connector;
99-
Map environment;
96+
private void connect () /* {{{ */
97+
{
98+
JMXServiceURL service_url;
99+
Map<String,Object> environment;
100100

101-
if (_jmx_connection != null)
102-
return;
101+
// already connected
102+
if (this._jmx_connector != null) {
103+
return;
104+
}
103105

104-
environment = null;
105-
if (this._password != null)
106-
{
107-
String[] credentials;
106+
environment = null;
107+
if (this._password != null)
108+
{
109+
String[] credentials;
108110

109-
if (this._username == null)
110-
this._username = new String ("monitorRole");
111+
if (this._username == null)
112+
this._username = new String ("monitorRole");
111113

112-
credentials = new String[] { this._username, this._password };
114+
credentials = new String[] { this._username, this._password };
113115

114-
environment = new HashMap ();
115-
environment.put (JMXConnector.CREDENTIALS, credentials);
116-
environment.put(JMXConnectorFactory.PROTOCOL_PROVIDER_CLASS_LOADER, this.getClass().getClassLoader());
117-
}
116+
environment = new HashMap<String,Object> ();
117+
environment.put (JMXConnector.CREDENTIALS, credentials);
118+
environment.put (JMXConnectorFactory.PROTOCOL_PROVIDER_CLASS_LOADER, this.getClass().getClassLoader());
119+
}
118120

119-
try
120-
{
121-
service_url = new JMXServiceURL (this._service_url);
122-
connector = JMXConnectorFactory.connect (service_url, environment);
123-
_jmx_connection = connector.getMBeanServerConnection ();
124-
}
125-
catch (Exception e)
121+
try
122+
{
123+
service_url = new JMXServiceURL (this._service_url);
124+
this._jmx_connector = JMXConnectorFactory.connect (service_url, environment);
125+
this._mbean_connection = _jmx_connector.getMBeanServerConnection ();
126+
}
127+
catch (Exception e)
128+
{
129+
Collectd.logError ("GenericJMXConfConnection: "
130+
+ "Creating MBean server connection failed: " + e);
131+
disconnect ();
132+
return;
133+
}
134+
} /* }}} void connect */
135+
136+
private void disconnect () /* {{{ */
126137
{
127-
Collectd.logError ("GenericJMXConfConnection: "
128-
+ "Creating MBean server connection failed: " + e);
129-
return;
130-
}
131-
} /* }}} void connect */
138+
try
139+
{
140+
this._jmx_connector.close();
141+
}
142+
catch (Exception e)
143+
{
144+
// It's fine if close throws an exception
145+
}
132146

133-
/*
134-
* public methods
135-
*
136-
* <Connection>
137-
* Host "tomcat0.mycompany"
138-
* ServiceURL "service:jmx:rmi:///jndi/rmi://localhost:17264/jmxrmi"
139-
* Collect "java.lang:type=GarbageCollector,name=Copy"
140-
* Collect "java.lang:type=Memory"
141-
* </Connection>
142-
*
143-
*/
147+
this._jmx_connector = null;
148+
this._mbean_connection = null;
149+
} /* }}} void disconnect */
150+
151+
/*
152+
* public methods
153+
*
154+
* <Connection>
155+
* Host "tomcat0.mycompany"
156+
* ServiceURL "service:jmx:rmi:///jndi/rmi://localhost:17264/jmxrmi"
157+
* Collect "java.lang:type=GarbageCollector,name=Copy"
158+
* Collect "java.lang:type=Memory"
159+
* </Connection>
160+
*
161+
*/
144162
public GenericJMXConfConnection (OConfigItem ci) /* {{{ */
145163
throws IllegalArgumentException
146164
{
@@ -217,9 +235,10 @@ public void query () /* {{{ */
217235
{
218236
PluginData pd;
219237

238+
// try to connect
220239
connect ();
221240

222-
if (this._jmx_connection == null)
241+
if (this._mbean_connection == null)
223242
return;
224243

225244
Collectd.logDebug ("GenericJMXConfConnection.query: "
@@ -234,11 +253,11 @@ public void query () /* {{{ */
234253
{
235254
int status;
236255

237-
status = this._mbeans.get (i).query (this._jmx_connection, pd,
256+
status = this._mbeans.get (i).query (this._mbean_connection, pd,
238257
this._instance_prefix);
239258
if (status != 0)
240259
{
241-
this._jmx_connection = null;
260+
disconnect ();
242261
return;
243262
}
244263
} /* for */

0 commit comments

Comments
 (0)