[
https://issues.apache.org/jira/browse/QPID-3289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13043284#comment-13043284
]
[email protected] commented on QPID-3289:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/833/#review752
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
<https://reviews.apache.org/r/833/#comment1567>
Using the setter but not the getter for a variable protected by its own
lock looks odd and at least deserves a comment as to why it is done this way
and why it is safe.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
<https://reviews.apache.org/r/833/#comment1568>
Minor point, but this patch adds several new cases of spurious trailing
whitespace. In this case it even adds to the noise and distracts from the real
changes.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
<https://reviews.apache.org/r/833/#comment1569>
The updating of the waitingOnSync flag above happens under a lock. Is this
supposed to be unlocked here?
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java
<https://reviews.apache.org/r/833/#comment1570>
A bit more explanation of the purpose of this check would be helpful. Is it
envisaged to be called concurrent to some other call to sync()? Or is this
detecting whether we are within a sync on the current thread? In the former
case you would seem to have a race condition which may or may not be ok. In the
latter there might be a nicer way to express it.
- Gordon
On 2011-06-02 02:52:57, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/833/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-06-02 02:52:57)
bq.
bq.
bq. Review request for qpid.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. As mentioned in the JIRA a simple solution is to not notify via the
exception listener if a JMS exception can be directly thrown to the application.
bq. The attached patch (unpolished) makes use of the sync call to determine if
an application can be notified directly or not.
bq. If it is then it will not notify via the connection listener.
bq. Andrew Kennedy had done some work previously to introduce a sync method in
AMQSession_0_10.java to facilitate sync calls.
bq. This patch builds on top of it to provide the above functionality.
bq.
bq. This patch also fixes issues like QPID-3259 and avoids other potential
locking issues due to the two ways(paths) of notifying the same error.
bq. Please note this patch only affects the 0-10 code path.
bq.
bq.
bq. This addresses bug QPID-3289.
bq. https://issues.apache.org/jira/browse/QPID-3289
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/AMQSession_0_10.java
1129752
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer_0_10.java
1129752
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_10.java
1129752
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
1129752
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java
1129752
bq.
bq. Diff: https://reviews.apache.org/r/833/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> Session exceptions should only be notified via the exception listener, if it
> cannot be thrown directly to the application.
> --------------------------------------------------------------------------------------------------------------------------
>
> Key: QPID-3289
> URL: https://issues.apache.org/jira/browse/QPID-3289
> Project: Qpid
> Issue Type: Bug
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
>
> The 0-10 code path in the JMS client always notifies a session exception via
> the connection listener, even when it can throw a JMS exception for a
> synchronous method call.
> For example session.createConsumer(destination) could fail due to an ACL
> violation and the session gets closed with an execution exception.
> Currently the JMS client throws an exception and also notifies the connection
> listener which results in the connection (and any other sessions associated
> with that connection) being closed.
> Another undesirable effect of this is the potential for deadlocks happening
> around the failovermutex.
> A reasonable solution for this problem would be to not notify via the
> connection listener if there is a way of throwing an exception to the
> application directly.
> For example session.createConsumer/Producer is a synchronous call and we
> could easily throw a JMS exception. In this case there is little value in
> notifying this via the connection listener and it also has the undesirable
> effect of closing the connection.
--
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:[email protected]
