[zeromq-dev] Correct behavior for closing messages after pipe::write in libzmq

[Topher Brown]

I recently tracked down a memory leak of the message memory buffer related
to disconnecting sockets. Github issue here:
https://github.com/zeromq/libzmq/issues/1313

The problem stemmed from not calling msg::close on a message that fails to
write to the pipe. While researching through the code I noticed
inconsistencies in how this scenario is handled, and I was hoping to come
to some kind of consencious on how to handle this issue.

My patch (https://github.com/zeromq/libzmq/pull/1314) only dealt with
cleaning up messages in router::xsend. However, the code in stream::xsend
looks nearly identical. Should it have the same fix? I have not done so
myself because I have no code that runs that function, although it is
pretty clear to me it should receive a similar patch.

The underlying question: when should msg::close be called? Should
msg::close be called after ALL calls to pipe::write (even successful ones)?
It looks like xsub::send_subscription uses the same paradigm as I
discovered with my patch (call msg::close only after failed pipe::write).
However router::xattach_pipe and dealer::xattach_pipe take the path of
msg::close in all cases (even on pipe::write success!).

Can we kick this problem back up the stack? Should pipe::write call
msg::close if it knows it is returning false (I think no)? How about this
then - should msg::init call msg::close on itself before resetting its
metadata? That would mitigate this issue and others like it, and can act as
a "failsafe" for not cleaning up messages properly in other places. After
all: who best to make sure messages are cleaned up than the message class
itself?

I'm hesitant to make these changes myself because I have only experienced
the one memory leak and I'm not super familiar with the libzmq internals.
I'd love to get guidance on the best way forward. I think at a minimum
stream::xsend should get a similar patch to mine to ensure non-sent
messages are closed properly. If people think it's a good idea, I'd be
happy to make a patch ensuring msg::close has been called in msg::init as
well.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.zeromq.org/pipermail/zeromq-dev/attachments/20150120/a657d58c/attachment.htm>