[ 
https://issues.apache.org/jira/browse/CXF-6454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15252210#comment-15252210
 ] 

ASF GitHub Bot commented on CXF-6454:
-------------------------------------

Github user dhpatel27 commented on the pull request:

    
https://github.com/apache/cxf/commit/5e3ac2b252412b90d6c91dea855773a294c3a565#commitcomment-17201043
  
    In 
rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSDestination.java:
    In 
rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSDestination.java
 on line 182:
    @cschneider #2 Problem mentioned in CXF Bug CXF-6454, I think we should 
still try to get rid of infinite loop problem as well. We are trying to give a 
feature of maxRetries as well so that customers can configure on their own as 
per their needs. Thus, I would still suggest we put a loop controller with 
maxRetries as in the original code from @vikash32504. Further there are 2 
problems in addition
    1. If a thread interrupts the loop, you are not cleaning up the resources 
done in method deactivate(). It simply is coming out without cleaning up the 
resources.
    2. We are still not giving control to the customer using this feature to 
control the maxRetries and coming out of the loop without interrupting the 
thread.
    3. Also, in addition to that, Thread.sleep() should be put in try block 
before deactivate as done in original commit by @vikash32504 since it does not 
make sense to retry immediately the JMS Connection. Thread.sleep will allow a 
grace period for Queue Mgr to come up and thus, we should keep Thread.sleep 
before deactivate and put maxRetries back in loop to come out of the endless 
loop



> Orphaned JMS connections created in endless loop
> ------------------------------------------------
>
>                 Key: CXF-6454
>                 URL: https://issues.apache.org/jira/browse/CXF-6454
>             Project: CXF
>          Issue Type: Bug
>          Components: JMS, Transports
>    Affects Versions: 3.0.5
>            Reporter: Waldemar Szostak
>            Assignee: Christian Schneider
>            Priority: Critical
>             Fix For: 3.0.7, 3.1.7, 3.2.0
>
>
> h3. Problem description
> In JMSFactory.createConnection(JMSConfiguration):
> {code}public static Connection createConnection(JMSConfiguration jmsConfig) 
> throws JMSException {
>       String username = jmsConfig.getUserName();
>       ConnectionFactory cf = jmsConfig.getConnectionFactory();
>       Connection connection = username != null 
>               ? cf.createConnection(username, jmsConfig.getPassword())
>               : cf.createConnection();
>       if (jmsConfig.getDurableSubscriptionClientId() != null) {
>               
> connection.setClientID(jmsConfig.getDurableSubscriptionClientId());
>       }
>       return connection;
> }{code}
> there is no exception handling if the clientID fails to be set. Such an 
> exception would exit this method without passing the reference to the 
> just-opened JMS connection to exception handling code 
> (JMSDestination.createTargetDestinationListener()).
> Moreover, JMSDestination.restartConnection() keeps on starting new 
> connections (there is no max attempt restriction!) until it creates one 
> without an exception thrown in the process.
> Now, if the clientID is already connected to the ESB, this creation of new 
> connection will last until server resources are no longer available to the 
> JVM.
> h3. Possible solution
> # Close the connection if it's not possible to set the specified clientID at 
> the time:
> {code}public static Connection createConnection(JMSConfiguration jmsConfig) 
> throws JMSException {
>       String username = jmsConfig.getUserName();
>       ConnectionFactory cf = jmsConfig.getConnectionFactory();
>       Connection connection = username != null 
>                       ? cf.createConnection(username, 
> jmsConfig.getPassword()) 
>                       : cf.createConnection();
>       if (jmsConfig.getDurableSubscriptionClientId() != null) {
>               try {                                   
> connection.setClientID(jmsConfig.getDurableSubscriptionClientId());
>               } catch (InvalidClientIDException e) {
>                       connection.close();
>                       throw e;
>               }
>       }
>       return connection;
> }{code}
> # Add a setting to restrict the maximum attempts to restart the connection in 
> JMSDestination.restartConnection() A configurable value would be best, but 
> even a hardcoded.. anything but the practically endless loop ;-)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to