----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36509/#review92100 -----------------------------------------------------------
Is the stack trace associated with this fix available anywhere? With a few possible exceptions, (e.g. running inside finalizers), doing an incref/decref should generally be safe and not need to be qualified by further conditions. The fact that it does need to be qualified implies that there may be a deeper bug here. That said I don't think this fix is unsafe so I wouldn't block it if it is an effective workaround, but it would be helpful to see the stack trace to figure out what is actually going on. - Rafael Schloming On July 15, 2015, 3:55 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36509/ > ----------------------------------------------------------- > > (Updated July 15, 2015, 3:55 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Bugs: PROTON-905 > https://issues.apache.org/jira/browse/PROTON-905 > > > Repository: qpid-proton-git > > > Description > ------- > > The original fix for PROTON-905, > https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;a=blobdiff;f=proton-c/src/engine/engine.c;h=fda719a3855e7bda9327148919c154499617c78a;hp=936cf609411d4ed0a8556227bfaf459b8fac753d;hb=653f4e5;hpb=bcfa8d44b8d71edd8c87568ebfd1141be175d054, > results in crashes in proton code during qpid-cpp tests. I believe this is > because the finalize call is triggered too early in some cases. The proposed > fix qualifies the previous fix to only apply in the case it was intended for. > > > Diffs > ----- > > proton-c/src/engine/engine.c fda719a > > Diff: https://reviews.apache.org/r/36509/diff/ > > > Testing > ------- > > All proton and qpid-cpp tests pass with this additional fix in place. > > > Thanks, > > Gordon Sim > >
