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

[Thomas Rodgers]

I've recently been looking at msg_t and metadata_t and the general lifetime
management issues around them.

zmq::msg_t is, in some sense, not a "proper" C++ class, in that while it
happily possesses all of the compiler generated default operations (default
ctor, copy ctor, default dtor, default assignment operator), none of it's
internals actually participate in ordinary C++ lifetime rules. I understand
the intention behind why msg_t works the way it does; you really do want
these things to have stack duration for small message sizes from an
efficiency standpoint, and you want the type to be opaque to the library
user.

Unfortunately, this has created problems
<https://github.com/zeromq/libzmq/issues/1058> before, with messages being
pushed onto a ypipe, and the internals of the message type not properly
participating in the invocation of operator=().

Because of this design, it is necessary for every single piece of code that
deals with msg_t to be aware of the lifecycle requirements of it's internal
state. In some case, the body has been passed off to some other code that
now 'owns' it, so it is ok to call init() on your local msg_t. In other
cases, you are the 'owner', and it is your responsibility to call close.

This to me seems messy and is only going to get worse with time (things get
added, memories get fuzzy, those that knew what was supposed to happen move
on, etc.).

I don't know if it is possible at this point to 'fix' this in any sort of
sane way, but I'm currently working on an experiment to see if the code can
be modified in such a way that C++'s normal lifetime semantics 'just work',
and the public API would still treat these as opaque stack local types and
work the way it always has from the user's perspective.

On Tue, Jan 20, 2015 at 4:49 PM, Topher Brown <topher200 at gmail.com> wrote:

> 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.
>
> _______________________________________________
> zeromq-dev mailing list
> zeromq-dev at lists.zeromq.org
> http://lists.zeromq.org/mailman/listinfo/zeromq-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.zeromq.org/pipermail/zeromq-dev/attachments/20150121/208eb999/attachment.htm>