----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16411/#review30740 -----------------------------------------------------------
Ship it! Looks good to me, thanks for fixing! One pedantic suggestion, would be to avoid calling reset() twice in normal reconnect path (and even less importantly avoid calling it if reconnect is not enabled). I don't think either of these could ever be a problem, just mentioning it as it is a slight deviation from what was there (yet not core to the fix I believe). See https://reviews.apache.org/r/16418/ for what I mean. Again, though, this is a pedantic tweak at most so feel free to ignore also. - Gordon Sim On Dec. 20, 2013, 2:11 a.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:11 a.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: QPID-5431 > https://issues.apache.org/jira/browse/QPID-5431 > > > Repository: qpid > > > Description > ------- > > 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. > > Minor improvements to drain.cpp and BrokerReplicator.cpp are not directly > relevant. > > > 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 > >
