[zeromq-dev] [PATCH]

[Daniel Sentenac]

Martin Sustrik wrote:
> Hi Daniel,
>> I've tried to apply your patch, however, I've realised there's a 
>> problem there. When you close the message, you may still use it in 
>> the next iteration, thus accessing deinitialised data.
>>
>> We have to think of something more sophisticated...
> Have a look at the attached patch. It's a patch for maint, however, 
> same thing should work for master. Can you check in your use case and 
> confirm it solves the leak? When you do so, I'll commit the patch to 
> both maint and master.
>
> Martin
> ------------------------------------------------------------------------
>
> diff --git a/src/pub.cpp b/src/pub.cpp
> index 4e73b19..d4a1824 100644
> --- a/src/pub.cpp
> +++ b/src/pub.cpp
> @@ -133,13 +133,25 @@ int zmq::pub_t::xsend (zmq_msg_t *msg_, int flags_)
>      }
>  
>      //  Push the message to all destinations.
> +    int passivised = 0;
>      for (pipes_t::size_type i = 0; i != active;) {
>          if (!write (pipes [i], msg_))
> -            content->refcnt.sub (1);
> +            passivised++;
>          else
>              i++;
>      }
>  
> +    //   If some of the pipes we've tried to send the message to have become
> +    //   passive we have to subtract corresponding number of references from
> +    //   the message. If there are no more references on the message we can
> +    //   destroy it.
> +    if (passivised) {
> +        if (!content->refcnt.sub (passivised)) {
> +            msg_->flags &= ~ZMQ_MSG_SHARED;
> +            zmq_msg_close (msg_);
> +        }
> +    }
> +
>      //  Detach the original message from the data buffer.
>      int rc = zmq_msg_init (msg_);
>      zmq_assert (rc == 0);
>   
Hi Martin,
I just tried your patch and I get a crash of the server starting from 
two receiver clients with following trace:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1282638944 (LWP 17487)]
0x00b07daa in memcpy () from /lib/tls/libc.so.6
(gdb) bt
#0  0x00b07daa in memcpy () from /lib/tls/libc.so.6
#1  0xb7e1ab0e in zmq::zmq_engine_t::out_event (this=0x8093b08) at 
encoder.hpp:117
#2  0xb7e01380 in zmq::epoll_t::loop (this=0x8088990) at epoll.cpp:193
#3  0xb7e013f3 in zmq::epoll_t::worker_routine (arg_=0x8088990) at 
epoll.cpp:210
#4  0xb7e15645 in zmq::thread_t::thread_routine (arg_=0x80889b0) at 
thread.cpp:99
#5  0x00c1d3cc in start_thread () from /lib/tls/libpthread.so.0
#6  0x00b65c3e in clone () from /lib/tls/libc.so.6

I think the problem comes from a useless  reference subtraction in the 
first test case, since you subtract all the passivised in a second time. 
I tried the following and it works well with no leak in my video 
distribution test:

 //  Push the message to all destinations.
    int passivised = 0;
    for (pipes_t::size_type i = 0; i != active;) {
      if (!write (pipes [i], msg_) )
    passivised++;
      else
    i++;
    }
 
    //   If some of the pipes we've tried to send the message to have become
    //   passive we have to subtract corresponding number of references from
    //   the message. If there are no more references on the message we can
    //   destroy it.
    if (passivised) {
      if (!content->refcnt.sub (passivised)) {
    msg_->flags &= ~ZMQ_MSG_SHARED;
    zmq_msg_close (msg_);
      }
    }