Alex Rousskov <[email protected]> writes: > On 01/21/2013 09:53 AM, Rainer Weikusat wrote: > >> + // XXX: connect method installed a write handler >> + // after a successful non-blocking connect. >> + // Since this handler is not going to be called >> + // and Comm::SetSelect is not (yet) 'smart pointer >> + // aware', the Pointer object needs to be >> + // destroyed explicitly here to prevent the ConnOpener >> + // object from being leaked. >> + // > > I do not think the above is 100% accurate, on two counts: We are > deleting ptr because we are about to remove our callback that deletes > the ptr. This is not directly related to SetSelect not being aware of > smart pointers (whatever that means) -- ptr is not smart. Also, it is > the ptr that leaks. The ConnOpener object leak is just a side effect of > the ptr leak. > > I suggest the following phrasing: > > XXX: We are about to remove write_handler, which was responsible for > deleting write_data, so we have to delete write_data ourselves. Comm > currently calls SetSelect handlers synchronously so if write_handler is > set, we know it has not been called yet. ConnOpener converts that sync > call into an async one, but only after deleting ptr, so that is not a > problem. > > If you agree that this phrasing is better,
To be honest, I don't really care: At worst, I would put this issue in some todo-list file but I wouldn't at comments describing it if this was entirely my own code. I just tried my best to distill what I understood from Amos' statement about the workaround, namely, 'ideally', just clearing the pointers via Comm::SetSelect would free anything which needs to be freed implicitly but since the data is ultimatively a void * and not some a pointer/ reference to an object of some class capable of 'freeing it' (the data), whatever 'it' happens to be, the explicit delete is still necessary at the moment, although this will hopefully change in future. You're - of course - right about the inaccuracy regarding the leak.
