You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2013/12/20 15:50:50 UTC

Re: Review Request 16411: Avoid double reset(), alternative to Gordon's suggestion.

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

(Updated Dec. 20, 2013, 2:50 p.m.)


Review request for qpid and Gordon Sim.


Changes
-------

Avoid double reset(), alternative to Gordon's suggestion.

I think this is a simpler fix, check the diff r1 r2. If checkDisconnected returns true then we know that reset() has already been called so no need to call it in check()' Do you see any flaws?


Summary (updated)
-----------------

Avoid double reset(), alternative to Gordon's suggestion.


Bugs: QPID-5431
    https://issues.apache.org/jira/browse/QPID-5431


Repository: qpid


Description (updated)
-------

Avoid double reset(), alternative to Gordon's suggestion.

I think this is a simpler fix, check the diff r1 r2. If checkDisconnected returns true then we know that reset() has already been called so no need to call it in check()' Do you see any flaws?


QPID-5431: Qpid c++ client hangs / crashes during reception failover in HA environment (mutual recursion)

Bug in AMQP 1.0 retry code caused an infinite recursion when failing over.
The recursion was in messaging::amqp::ConnectionContext, where the following
recursive cycle could occur:
 check()->autoconnect()->tryConnect(Url)->tryConnect(Address)->wait()->check()->...

Re-organized the code to avoid the recursion, specifically avoid calling check()
in tryConnect(Address). A disconnect detected in tryConnect results in continuing
the retry rather than calling autoconnect again.


Diffs (updated)
-----

  /trunk/qpid/cpp/examples/messaging/drain.cpp 1552476 
  /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1552476 
  /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1552476 
  /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1552476 

Diff: https://reviews.apache.org/r/16411/diff/


Testing
-------

Bug reproducer, full make test.


Thanks,

Alan Conway


Re: Review Request 16411: Avoid double reset(), alternative to Gordon's suggestion.

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16411/#review30746
-----------------------------------------------------------

Ship it!


Ship It!

- Gordon Sim


On Dec. 20, 2013, 2:50 p.m., Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16411/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2013, 2:50 p.m.)
> 
> 
> Review request for qpid and Gordon Sim.
> 
> 
> Bugs: QPID-5431
>     https://issues.apache.org/jira/browse/QPID-5431
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Avoid double reset(), alternative to Gordon's suggestion.
> 
> I think this is a simpler fix, check the diff r1 r2. If checkDisconnected returns true then we know that reset() has already been called so no need to call it in check()' Do you see any flaws?
> 
> 
> QPID-5431: Qpid c++ client hangs / crashes during reception failover in HA environment (mutual recursion)
> 
> Bug in AMQP 1.0 retry code caused an infinite recursion when failing over.
> The recursion was in messaging::amqp::ConnectionContext, where the following
> recursive cycle could occur:
>  check()->autoconnect()->tryConnect(Url)->tryConnect(Address)->wait()->check()->...
> 
> Re-organized the code to avoid the recursion, specifically avoid calling check()
> in tryConnect(Address). A disconnect detected in tryConnect results in continuing
> the retry rather than calling autoconnect again.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/examples/messaging/drain.cpp 1552476 
>   /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1552476 
>   /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1552476 
>   /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1552476 
> 
> Diff: https://reviews.apache.org/r/16411/diff/
> 
> 
> Testing
> -------
> 
> Bug reproducer, full make test.
> 
> 
> Thanks,
> 
> Alan Conway
> 
>


Re: Review Request 16411: Avoid double reset(), alternative to Gordon's suggestion.

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16411/#review30747
-----------------------------------------------------------

Ship it!


Ship It!

- Gordon Sim


On Dec. 20, 2013, 2:50 p.m., Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16411/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2013, 2:50 p.m.)
> 
> 
> Review request for qpid and Gordon Sim.
> 
> 
> Bugs: QPID-5431
>     https://issues.apache.org/jira/browse/QPID-5431
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Avoid double reset(), alternative to Gordon's suggestion.
> 
> I think this is a simpler fix, check the diff r1 r2. If checkDisconnected returns true then we know that reset() has already been called so no need to call it in check()' Do you see any flaws?
> 
> 
> QPID-5431: Qpid c++ client hangs / crashes during reception failover in HA environment (mutual recursion)
> 
> Bug in AMQP 1.0 retry code caused an infinite recursion when failing over.
> The recursion was in messaging::amqp::ConnectionContext, where the following
> recursive cycle could occur:
>  check()->autoconnect()->tryConnect(Url)->tryConnect(Address)->wait()->check()->...
> 
> Re-organized the code to avoid the recursion, specifically avoid calling check()
> in tryConnect(Address). A disconnect detected in tryConnect results in continuing
> the retry rather than calling autoconnect again.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/examples/messaging/drain.cpp 1552476 
>   /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1552476 
>   /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.h 1552476 
>   /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1552476 
> 
> Diff: https://reviews.apache.org/r/16411/diff/
> 
> 
> Testing
> -------
> 
> Bug reproducer, full make test.
> 
> 
> Thanks,
> 
> Alan Conway
> 
>