[GitHub] [cxf] steingebein commented on issue #603: [CXF-8161] fix memory leak and thread leak in JMSDestination

[GitBox <gi...@apache.org>]

steingebein commented on issue #603: [CXF-8161] fix memory leak and thread leak in JMSDestination
URL: https://github.com/apache/cxf/pull/603#issuecomment-563237415
 
 
   Hi @reta,
   
   I think your idea is better than the current pull request. It moves the fix completly into `JMSDestination` and it will work even if you replace or change `PollingMessageListenerContainer` 
   
   But I think it comes up with one problem: We have to revert my change in `restartConnection()`, which checks `jmsListener.isRunning()` before restarting a connection. Otherwise `restartConnection()` does nothing (`PollingMessageListenerContainer.running` is not unset).
   It will cause every short-lived thread from `ExceptionListener` to stop and restart the connection. You get `jmsConfig.getConcurrentConsumers()` reconnects.
   
   Perhaps it's better to restart only once per `PollingMessageListenerContainer`/`ExceptionListener`-instance:
   ```
   ExceptionListener exListener = new ExceptionListener() {
                   private boolean restartTriggered = false;
                   public synchronized void onException(JMSException exception) {
                       if (!shutdown && !restartTriggered) {
                           LOG.log(Level.WARNING, "Exception on JMS connection. Trying to reconnect", exception);
                           new Thread(new Runnable() {
                                @Override
                                public void run() {
                                    restartConnection();
                                }
                           }).start();
                           restartTriggered = true;
                       }
                   }
               };
   ```
   changes:
   - removed your if-condition, because `jmsListener == null` is always false when `onException` is called
   - `restartTriggered` prevents multiple restarts from the `PollingMessageListenerContainer`-threads
   - `synchronized onException` because it's called from multiple threads
   
   
    
    

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services