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

jirapos...@reviews.apache.org commented on QPID-3269:
-----------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1027/#review989
-----------------------------------------------------------


This seems a bit horrible, deliberately leaving a session open and unused. Were 
there really no alternative options?

On first glance I also don't imagine this works with the Java broker, which 
would admittedly be the brokers fault but has it been tested? There is a 
MaxChannels value that gets negotiated with the broker during the AMQP 
connection, and the the client currently uses it to provide feedback to users 
when they exceed the allowed number of JMS Sessions, I think this will probably 
break that and allow more Sessions to be created/attempted than should be.

When the clientid is being verified the method declares it throws JMSException 
from the delegate, but Exception is caught by the calling method instead. Would 
a boolean return not suffice here, with exceptions only being thrown from the 
verification method due to unexpected occurences?

- Robbie


On 2011-07-07 02:17:42, rajith attapattu wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1027/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-07-07 02:17:42)
bq.  
bq.  
bq.  Review request for qpid, Gordon Sim and Robbie Gemmell.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  In order to verify the uniqueness of the client ID, a dummy session is 
created using client ID as it's name. This prevents any other connection from 
using same client ID as the session creation will fail. However this 
verification is switched off by default in order to preserve backwards 
compatibility. You need to use -Dqpid.verify_client_id=true switch verification 
on.
bq.  
bq.  In summary the following changes were made in order to support the above,
bq.  1. A verifyClientID method was added to the connection delegates,
bq.  2. AMQSession_0_10.java was modified to allow a name to be specified for 
the underlying AMQP session.
bq.  3. A method was added to o.a.q.transport.Session.java to wait until the 
session state was changed from NEW to OPEN (or another state which triggers the 
error).
bq.  4. Setter/Getter in Session.java to store/retrieve the SessionDetachCode 
and ConnectionDelegate to set the detach code.
bq.  5. SessionDelegate to notify Session object when attached/dettached/closed 
is invoked.
bq.  
bq.  
bq.  This addresses bug QPID-3269.
bq.      https://issues.apache.org/jira/browse/QPID-3269
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
 1143628 
bq.    
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java
 1143628 
bq.  
bq.  Diff: https://reviews.apache.org/r/1027/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  A patch containing a test will be attached to the JIRA shortly.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  rajith
bq.  
bq.



>  Concurrently executing connections are allowed to use the same client ID
> -------------------------------------------------------------------------
>
>                 Key: QPID-3269
>                 URL: https://issues.apache.org/jira/browse/QPID-3269
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Client
>    Affects Versions: 0.6, 0.8, 0.10
>            Reporter: Rajith Attapattu
>            Assignee: Rajith Attapattu
>            Priority: Minor
>             Fix For: 0.11
>
>
> Section 4.3.2 of the JMS spec says that the JMS provider 
> should prevent concurrently executing clients from using the same client id.
> Qpid JMS client allows two connections to be created using the same client id.
> How reproducible:
> Always
> Steps to Reproduce:
> 1. Create two connections with the same client ID.
> 2. Observe that there are no exceptions being thrown.
> Actual results:
> No exception being thrown.
> Expected results:
> An exception should be thrown.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org

Reply via email to