steingebein commented on a change in pull request #603: [CXF-8161] fix memory
leak and thread leak in JMSDestination
URL: https://github.com/apache/cxf/pull/603#discussion_r350828798
##########
File path:
rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSDestination.java
##########
@@ -174,9 +174,9 @@ public void onException(JMSException exception) {
}
}
- protected void restartConnection() {
+ protected synchronized void restartConnection() {
Review comment:
I have one suggestion to improve the code:
Move `sychnronized` from `JMSDestination#restartConnection()` to
`PollingMessageListenerContainer#handleException()`, because
`PollingMessageListenerContainer` starts multiple threads and needs the
synchronisation. So `sychnronized` belongs more into this class.
To complete this change, I have to add `!jmsListener.isRunning() &&` in
`JMSDestination#activate()`:
```
if (!jmsListener.isRunning() && !jmsConfig.isOneSessionPerConnection()) {
// If first connect fails we will try to establish the
connection in the background
new Thread(new Runnable() {
@Override
public void run() {
restartConnection();
}
}).start();
}
```
But I'm not sure if it makes the code better...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
