Possibly incorrect gr_msg_queue.delete_head_nowait() implementation

According to the documentation comments,
gr_msg_queue::delete_head_nowait()
should return 0 if no message is available. The current implementation,
however, results in a core dump.

For example, when executed in a script, the eleven lines:

#/usr/bin/env python
from gnuradio import gr
q = gr.msg_queue(1)
msg1_in = gr.message_from_string(‘msg1 payload’)
q.insert_tail(msg1_in)
msg1_out = q.delete_head_nowait()
print ‘msg1_out object: %s’ % str(msg1_out)
print ‘msg1_out value: %s’ % msg1_out.to_string()
msg2_out = q.delete_head_nowait()
print ‘msg2_out object: %s’ % str(msg2_out)
print ‘msg2_out value: %s’ % msg2_out.to_string()

generate the output:

msg1_out object: <gnuradio.gr.gnuradio_core_runtime.gr_message_sptr;
proxy
of <Swig Object of type ‘gr_message_sptr *’ at 0x16529c0> >
msg1_out value: msg1 payload
msg2_out object: <gnuradio.gr.gnuradio_core_runtime.gr_message_sptr;
proxy
of <Swig Object of type ‘gr_message_sptr ’ at 0x16529f0> >
python: /usr/include/boost/smart_ptr/shared_ptr.hpp:418: T

boost::shared_ptr< >::operator->() const [with
T =
gr_message]: Assertion `px != 0’ failed.
Aborted (core dumped)

All of the other gr.msg_queue swig’d functions also cause a core dump.

The definition of gr_msg_queue:delete_head_nowait() includes the block:

if ((m = d_head) == 0){
//return 0;
return gr_message_sptr();
}

I believe the correct implementation should the ‘return 0’ line
uncommented
and the ‘return gr_message_sprt()’ removed.

This should have been obvious to me earlier. The problem is that 0 is
being return and the assertion that ‘px != 0’ (correctly) fails. Perhaps
the use of gr.msg_queue.delete_head_nowait() should be deprecated and
the
equivalent behavior achieved by checking if gr.msg_queue.count() == 0
prior
to calling gr.msg_queue.delete_head().

On Thu, Oct 3, 2013 at 3:33 PM, Isdren Gineer <

Of course, if 0 is returned, msg2_out would not be an message instance,
but
it would be possible to check if msg2_out == 0 before trying to call any
instance methods.

On Thu, Oct 3, 2013 at 3:27 PM, Isdren Gineer <

Hi Isdren,

I believe the correct implementation should the ‘return 0’ line uncommented and
the ‘return gr_message_sprt()’ removed.

That would not compile, since you can’t convert int to
boost::shared_ptr.
What you see in your python output are really only the swig wrappers of
these shared pointers including their memory address, not the address
they points at.
So yes, if you use a _nowait() method, you usually have to check its
output; that’s the price you pay for not waiting as long as it takes to
produce your return value.

As far as I know, there is no way to check the address that a shared_ptr
points add from within python; I’m not quite sure there is an easy way
to fix this. Swig just doesn’t work like that, and extending
boost::shared_ptr doesn’t sound very sensible.
I guess a logical way would be to have a c++ implementation of the
equality comparison operator that takes two
boost::shared_ptrgr::message . That would make it possible to check
wether a sptr actually points to an object or is a nullpointer from
python. However, that’d be a really “ugly” thing from a syntax point of
view:

p2 = q.delete_head_nowait()
p_ref = gr.message_sptr()
if not gr.message.sptr_equals(p2,p_ref):
print p2.to_string()

You further wrote:

This should have been obvious to me earlier. The problem is that 0
is being return and the assertion that ‘px != 0’ (correctly) fails.
Perhaps the use of gr.msg_queue.delete_head_nowait() should be
deprecated and the equivalent behavior achieved by checking if
gr.msg_queue.count() == 0 prior to calling gr.msg_queue.delete_head().

I strongly disagree - the delete_nowait_method has it’s uses and is part
of the standard scheduler; it’s generally a good idea to have a
non-blocking pop for a mutexed queue, otherwise it can be really hard to
avoid deadlock conditions.
Since your “if gr.msg_queue.count() == 0: p2
=gr.msg_queue.delete_head()” is not atomic, it would not even fix the
issue - someone else might still be stealing your last queue element
between your check and your delete_head().

Greetings,
Marcus