> On 2011-07-07 12:14:51, Gordon Sim wrote: > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java, > > line 1069 > > <https://reviews.apache.org/r/1027/diff/1/?file=22094#file22094line1069> > > > > Could you not have used sync() here instead of a new method? > > rajith attapattu wrote: > I did wonder about this myself. But once a session is detached you can no > longer sync on it right (from the broker side) ? (Bcos you need a valid > session to sync on). > Besides when the session is detached it's marked close on the client > side, so sync wouldn't work anyways as we will throw a session closed > exception.
The point would be though that if you are sync()ing on the client side and the session is detached, that already wakes you up and throws an exception, right? I don't understand the second sentence. However your new method is throwing a session closed exception as well. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1027/#review985 ----------------------------------------------------------- On 2011-07-07 02:17:42, rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1027/ > ----------------------------------------------------------- > > (Updated 2011-07-07 02:17:42) > > > Review request for qpid, Gordon Sim and Robbie Gemmell. > > > Summary > ------- > > 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. > > In summary the following changes were made in order to support the above, > 1. A verifyClientID method was added to the connection delegates, > 2. AMQSession_0_10.java was modified to allow a name to be specified for the > underlying AMQP session. > 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). > 4. Setter/Getter in Session.java to store/retrieve the SessionDetachCode and > ConnectionDelegate to set the detach code. > 5. SessionDelegate to notify Session object when attached/dettached/closed is > invoked. > > > This addresses bug QPID-3269. > https://issues.apache.org/jira/browse/QPID-3269 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java > 1143628 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java > 1143628 > > Diff: https://reviews.apache.org/r/1027/diff > > > Testing > ------- > > A patch containing a test will be attached to the JIRA shortly. > > > Thanks, > > rajith > >
