----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17329/#review32832 -----------------------------------------------------------
/proton/trunk/proton-c/src/engine/engine.c <https://reviews.apache.org/r/17329/#comment61839> I'm putting this comment here because it kind of relates to changing the pn_free to a pn_decref, but really it's about the overall diff. Assuming I'm reading it correctly, I believe you have modified the ref counting semantics such that holding a pointer to a child object prevents the parent from being collected. While this makes sense from a python perspective, this actually makes the C API far more brittle since I can't simply free the connection and be sure that it and all of its components go away. I need to ensure that I track and free every single child object that I've created. This seems kind of annoying since the connection is tracking all that stuff anyways. I thought the idea was that the connection level free blows away everything, but that the python binding keeps parent pointers to prevent the connection from being blown away. That said, I don't think it's necessarily bad to implement these kind of semantics in C if it means less work in the bindings, however I think there should still be a connection level free that blows everything away if only for situations in C where you want to robustly clean up. - Rafael Schloming On Jan. 27, 2014, 1:33 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17329/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2014, 1:33 p.m.) > > > Review request for qpid, Gordon Sim and Rafael Schloming. > > > Bugs: proton-489 > https://issues.apache.org/jira/browse/proton-489 > > > Repository: qpid > > > Description > ------- > > Prior to this fix, freeing an endpoint would leave stale pointers in > referring objects. This fix makes use of the object reference functionality > to ensure an endpoint is only removed when all references to it are dropped. > > > Diffs > ----- > > /proton/trunk/proton-c/include/proton/object.h 1560765 > /proton/trunk/proton-c/src/engine/engine.c 1560765 > /proton/trunk/proton-c/src/messenger/messenger.c 1560765 > /proton/trunk/proton-c/src/object/object.c 1560765 > /proton/trunk/proton-c/src/transport/transport.h 1560765 > /proton/trunk/proton-c/src/transport/transport.c 1560765 > > Diff: https://reviews.apache.org/r/17329/diff/ > > > Testing > ------- > > Unit tests, including valgrind tests. > > > Thanks, > > Kenneth Giusti > >
