> On March 27, 2013, 3:53 p.m., Kenneth Giusti wrote:
> > Looks fine - but I'm not a big fan of the xxxLH() convention, though. It's
> > too brittle. A code change that (re)moves the lock will cause the code to
> > fail, but cannot be detected by the compiler. Not much of an issue if the
> > developer is careful, but a real problem when backporting patches.
> >
> > My preference - while not a very popular one - is to change the parameter
> > list of the xxxLH() call to take a const reference to the held lock, while
> > the actual function ignores this parameter. This allows the compiler to
> > enforce that there is a lock around the xxLH() call, since the lock
> > actually has to be passed. Should a patch be applied that (re)moves the
> > lock, the compilation will fail.
> >
> > Example:
> >
> > ScopedLock l(somelock);
> > ...
> > xxxLH( param1, param2, ..., l );
> >
> > with the definition of xxxLH() ignoring that parameter:
> >
> > void xxxLH( int param1, int param2, ... const ScopedLock& /*ignored*/ ) {
> > ....
> >
> >
> > I can see why most people don't like this convention - it implies the
> > method may modify the lock - but the const designation prevents that.
> >
> > That said, I'm fine with the patch as-is.
+1
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10155/#review18425
-----------------------------------------------------------
On March 27, 2013, 2:38 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10155/
> -----------------------------------------------------------
>
> (Updated March 27, 2013, 2:38 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Kenneth Giusti.
>
>
> Description
> -------
>
> Self test federation.test_dynamic_topic hangs. Two threads take out an Rlock
> and then try to upgrade the lock to a Wlock and hang.
>
> The failure was intermittent. Only automake builds running 'make check'
> seemed to have the issue.
>
> The fix is to create an unbindLH() function that does the unbind but does not
> take out the Wlock itself.
>
> The note in the Jira about similar behavior in the TopicExchange is wrong;
> this issue is XmlExchange only.
>
>
> This addresses bug QPID-4672.
> https://issues.apache.org/jira/browse/QPID-4672
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1461592
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1461592
>
> Diff: https://reviews.apache.org/r/10155/diff/
>
>
> Testing
> -------
>
> Passes automake and cmake tests.
>
>
> Thanks,
>
> Chug Rolke
>
>
