----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18900/#review36504 -----------------------------------------------------------
Ship it! I think it is probably ok. The destroy doesn't actually delete the exchange object (that is controlled through shared_ptrs), it merely deletes it from the registry and signals deletion through management. Should be the same as a manual delete while the exchange is in use. /trunk/qpid/cpp/src/qpid/broker/Exchange.cpp <https://reviews.apache.org/r/18900/#comment67496> Perhaps: if (autodelete) { if (isControllingLink) { if (broker) broker->getExchanges().destroy(name); } else if (!inUse() && !hasBindings()) { checkAutoDelete(); } } is a bit clearer(?), but that is likely subjective. - Gordon Sim On March 7, 2014, 9:49 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18900/ > ----------------------------------------------------------- > > (Updated March 7, 2014, 9:49 a.m.) > > > Review request for qpid, Gordon Sim and Kenneth Giusti. > > > Bugs: QPID-5608 > https://issues.apache.org/jira/browse/QPID-5608 > > > Repository: qpid > > > Description > ------- > > The fix follows how IncomingToQueue class behave here for queues. Potential > issues with the fix (causing me to raise the review request before committing > it): > > 1) change in Exchange::decOtherUsers - cant it be done in more elegant way? > 2) can't the petch in general cause problems to some other lifetime policies? > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Exchange.h 1574030 > /trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1574030 > /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1574030 > > Diff: https://reviews.apache.org/r/18900/diff/ > > > Testing > ------- > > automated tests seem passed, the reproducer from JIRA is fixed > > > Thanks, > > Pavel Moravec > >
