You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Phil Harvey <ph...@philharveyonline.com> on 2013/01/23 15:19:41 UTC

Question about proton-c message id API

I'm working on the proton-c JNI binding and am having trouble with the
Message.getMessageId() JNI implementation.

I am currently calling pn_message_get_id but this is causing a segfault
(details below).  I suspect that I might be using the proton-c message API
incorrectly and would appreciate advice from someone more familiar with the
proton-c implementation.

Our call stack is:
AccessorsTest.testId
  ...
    Message._get_id
      JNIMessage.getMessageId()
        ...
          pn_message_get_id
            pn_data_get_atom
              <segfaults because node is null>

This suggests that our problem is that we haven't previously called
pn_message_set_id.

message.h contains the following:

pn_data_t *    pn_message_id                    (pn_message_t *msg);
pn_atom_t      pn_message_get_id                (pn_message_t *msg);
int            pn_message_set_id                (pn_message_t *msg,
pn_atom_t id);

The JNI implementation of Message.getMessageId() calls pn_message_get_id.
However, in revision 1400309 (committed by Rafi) the proton-c Python
binding was modified to implement this using the pn_data returned from
pn_message_id instead.

Are the pn_message_get_id and pn_message_set_id functions deprecated?  If
so, the Java Message interface should possibly have its getMessageId and
setMessageId methods removed, and replaced with calls to a new interface
(called MessageId?).

If not, can anyone explain the intended interplay between the three message
id functions?

Finally, it seems wrong to me that pn_message_get_id should ever cause a
seg fault.  Is this a bug?

Phil

Re: Question about proton-c message id API

Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Thu, Jan 24, 2013 at 7:38 AM, Keith W <ke...@gmail.com> wrote:

> On 23 January 2013 15:40, Rafael Schloming <rh...@alum.mit.edu> wrote:
> > Yeah, it appears to be a bug. I checked in a potential fix on trunk. Give
> > it a shot and see if it's still an issue.
> >
> > --Rafael
>
> Thanks, that has indeed addressed the issue.  The Message system tests
> which previously failed now run cleanly against the Proton-JNI (on the
> jni-branch).
>
> It does leave the question regarding the differences in usage of the
> Proton-API. The Python/C binding makes calls to pn_message,
> pn_message_id, and pn_message_correlation_id when creating its Message
> facade object.  The Ruby/C and Java/C bindings call only pn_message
> during construction of their facade object.
>
> Python/C binding:
>
> def __init__(self):
>   self._msg = pn_message()
>   self._id = Data(pn_message_id(self._msg))
>   self._correlation_id = Data(pn_message_correlation_id(self._msg))
>
> Ruby/C binding:
>
> def initialize
>   @impl = Cproton.pn_message
>   ObjectSpace.define_finalizer(self, self.class.finalize!(@impl))
>   end
>
> Java/C binding (jni-branch only)
>
> JNIMessage()
> {
>   _impl = Proton.pn_message();
> }
>
> Is there any reason why Python/C works in a different manner?
>

It's just to avoid code duplication between the id property's getter and
setter:

  def _get_id(self):
    return self._id.get_object()
  def _set_id(self, value):
    if type(value) in (int, long):
      value = ulong(value)
    self._id.rewind()
    self._id.put_object(value)
  id = property(_get_id, _set_id,
                doc="""
The id of the message.
""")

I don't think this particular difference should have an impact on behaviour
one way or another. I think the reason the python binding wasn't hitting
the same bug you've observed is that it extracts the id from the data
object by calling Data.get_object, and that will first query the type of
the current node and then call a specific accessor based on that type,
whereas you appear to be calling pn_data_get_atom which will return the
type and the value in a discriminated union. It's possible we could
simplify the swig binding process by always using the former style as then
we might be able to avoid having to add typemaps for pn_atom_t.

--Rafael

Re: Question about proton-c message id API

Posted by Keith W <ke...@gmail.com>.
On 23 January 2013 15:40, Rafael Schloming <rh...@alum.mit.edu> wrote:
> Yeah, it appears to be a bug. I checked in a potential fix on trunk. Give
> it a shot and see if it's still an issue.
>
> --Rafael

Thanks, that has indeed addressed the issue.  The Message system tests
which previously failed now run cleanly against the Proton-JNI (on the
jni-branch).

It does leave the question regarding the differences in usage of the
Proton-API. The Python/C binding makes calls to pn_message,
pn_message_id, and pn_message_correlation_id when creating its Message
facade object.  The Ruby/C and Java/C bindings call only pn_message
during construction of their facade object.

Python/C binding:

def __init__(self):
  self._msg = pn_message()
  self._id = Data(pn_message_id(self._msg))
  self._correlation_id = Data(pn_message_correlation_id(self._msg))

Ruby/C binding:

def initialize
  @impl = Cproton.pn_message
  ObjectSpace.define_finalizer(self, self.class.finalize!(@impl))
  end

Java/C binding (jni-branch only)

JNIMessage()
{
  _impl = Proton.pn_message();
}

Is there any reason why Python/C works in a different manner?

Kind regards, Keith.

Re: Question about proton-c message id API

Posted by Rafael Schloming <rh...@alum.mit.edu>.
Yeah, it appears to be a bug. I checked in a potential fix on trunk. Give
it a shot and see if it's still an issue.

--Rafael

On Wed, Jan 23, 2013 at 9:19 AM, Phil Harvey <ph...@philharveyonline.com>wrote:

> I'm working on the proton-c JNI binding and am having trouble with the
> Message.getMessageId() JNI implementation.
>
> I am currently calling pn_message_get_id but this is causing a segfault
> (details below).  I suspect that I might be using the proton-c message API
> incorrectly and would appreciate advice from someone more familiar with the
> proton-c implementation.
>
> Our call stack is:
> AccessorsTest.testId
>   ...
>     Message._get_id
>       JNIMessage.getMessageId()
>         ...
>           pn_message_get_id
>             pn_data_get_atom
>               <segfaults because node is null>
>
> This suggests that our problem is that we haven't previously called
> pn_message_set_id.
>
> message.h contains the following:
>
> pn_data_t *    pn_message_id                    (pn_message_t *msg);
> pn_atom_t      pn_message_get_id                (pn_message_t *msg);
> int            pn_message_set_id                (pn_message_t *msg,
> pn_atom_t id);
>
> The JNI implementation of Message.getMessageId() calls pn_message_get_id.
> However, in revision 1400309 (committed by Rafi) the proton-c Python
> binding was modified to implement this using the pn_data returned from
> pn_message_id instead.
>
> Are the pn_message_get_id and pn_message_set_id functions deprecated?  If
> so, the Java Message interface should possibly have its getMessageId and
> setMessageId methods removed, and replaced with calls to a new interface
> (called MessageId?).
>
> If not, can anyone explain the intended interplay between the three message
> id functions?
>
> Finally, it seems wrong to me that pn_message_get_id should ever cause a
> seg fault.  Is this a bug?
>
> Phil
>