You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@qpid.apache.org by Ken Giusti <kg...@redhat.com> on 2015/03/09 15:35:16 UTC

proton application context API deprecated in 0.9?

Hi, 

I just noticed that the 0.9rc1 marks the pn_XXX_get_context()/pn_XXX_set_context() set of APIs as being deprecated.

I use these apis fairly frequently as a means to map back to my application's context.

What are they being replaced with?   I couldn't find an associated JIRA explaining how to upgrade my code.

thanks,

-- 
-K

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: proton application context API deprecated in 0.9?

Posted by Ken Giusti <kg...@redhat.com>.

----- Original Message -----
> From: "Rafael Schloming" <rh...@alum.mit.edu>
> To: users@qpid.apache.org
> Sent: Tuesday, March 10, 2015 6:44:29 AM
> Subject: Re: proton application context API deprecated in 0.9?
> 
> On Mon, Mar 9, 2015 at 4:11 PM, Ken Giusti <kg...@redhat.com> wrote:
> 
> > Hi Rafi - thank you for that description.  I'll have to dig into that code
> > a bit more to get a feel for it.
> >
> > The only concern I have is the implementation of PN_HANDLE.  If I'm
> > correct, you can't directly share PN_HANDLE's across compilation units due
> > to the use of static variables. In other words, if I have
> >
> > PN_HANDLE(RAFI);
> >
> > in my rafi_private.h header, each compilation unit will define a RAFI
> > handle with a different underlying value.
> >
> 
> That's a good point, however it shouldn't be a problem for the intended
> usage because the macro would never ever appear in a .h file, only .c files.
> 

But nothing (currently) prevents doing that, unless you've spent time reading the header files and understand the implementation.  Speaking personally, it would be better to idiot proof this :)

> 
> > Since it's not a compilation-time error, this could result in a very hard
> > to diagnose run time bug.
> >
> > Perhaps a simple #defined constant would be safer?
> >
> 
> The trouble with using a simple constant is that two independent pieces of
> code might end up choosing the same constant resulting in similarly subtle
> bugs. The PN_HANDLE macro gives independent pieces of code a way to define
> guaranteed unique constants without having to depend on some sort of
> central allocation strategy that needs to be initialized. The language
> runtime provides the guarantees based on memory location.
> 
> I think if you follow the best practice strategy of always defining
> pn_xxx_get/set_foo accessors, then not only will your code be type safe,
> but you should also never need to use a handle outside of the one
> compilation unit that defines the accessors. This would even be a good idea
> if you were using simple #define constants since you would have a much
> safer ABI. We could probably even enforce this pattern by putting something
> in the macro that would barf if you were to ever stick it in a .h file and
> include it twice. (Maybe even just making the declarations non-static would
> be sufficient.)
> 

I'm good with simply changing the definition of PN_HANDLE() slightly:

#define PN_HANDLE(name)                 \
  static char *_PN_HANDLE_ ## name = 0; \
  const pn_handle_t name = ((pn_handle_t) &_PN_HANDLE_ ## name);

so there's at least link-time enforcement of the singleton (and error messages will complain about $name, not _PN_HANDLE_$name).

If you wanted to be even more explicit as to the pattern, you could add the following:

#define PN_HANDLE_EXTERN(name)  extern const pn_handle_t name;

which would be an additional usage hint.


> --Rafael
> 

-- 
-K

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: proton application context API deprecated in 0.9?

Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Mon, Mar 9, 2015 at 4:11 PM, Ken Giusti <kg...@redhat.com> wrote:

> Hi Rafi - thank you for that description.  I'll have to dig into that code
> a bit more to get a feel for it.
>
> The only concern I have is the implementation of PN_HANDLE.  If I'm
> correct, you can't directly share PN_HANDLE's across compilation units due
> to the use of static variables. In other words, if I have
>
> PN_HANDLE(RAFI);
>
> in my rafi_private.h header, each compilation unit will define a RAFI
> handle with a different underlying value.
>

That's a good point, however it shouldn't be a problem for the intended
usage because the macro would never ever appear in a .h file, only .c files.


> Since it's not a compilation-time error, this could result in a very hard
> to diagnose run time bug.
>
> Perhaps a simple #defined constant would be safer?
>

The trouble with using a simple constant is that two independent pieces of
code might end up choosing the same constant resulting in similarly subtle
bugs. The PN_HANDLE macro gives independent pieces of code a way to define
guaranteed unique constants without having to depend on some sort of
central allocation strategy that needs to be initialized. The language
runtime provides the guarantees based on memory location.

I think if you follow the best practice strategy of always defining
pn_xxx_get/set_foo accessors, then not only will your code be type safe,
but you should also never need to use a handle outside of the one
compilation unit that defines the accessors. This would even be a good idea
if you were using simple #define constants since you would have a much
safer ABI. We could probably even enforce this pattern by putting something
in the macro that would barf if you were to ever stick it in a .h file and
include it twice. (Maybe even just making the declarations non-static would
be sufficient.)

--Rafael

Re: proton application context API deprecated in 0.9?

Posted by Ken Giusti <kg...@redhat.com>.
Hi Rafi - thank you for that description.  I'll have to dig into that code a bit more to get a feel for it.

The only concern I have is the implementation of PN_HANDLE.  If I'm correct, you can't directly share PN_HANDLE's across compilation units due to the use of static variables. In other words, if I have

PN_HANDLE(RAFI);

in my rafi_private.h header, each compilation unit will define a RAFI handle with a different underlying value.

Since it's not a compilation-time error, this could result in a very hard to diagnose run time bug.

Perhaps a simple #defined constant would be safer?

----- Original Message -----
> From: "Rafael Schloming" <rh...@alum.mit.edu>
> To: users@qpid.apache.org
> Sent: Monday, March 9, 2015 3:10:47 PM
> Subject: Re: proton application context API deprecated in 0.9?
> 
> On Mon, Mar 9, 2015 at 10:35 AM, Ken Giusti <kg...@redhat.com> wrote:
> 
> > Hi,
> >
> > I just noticed that the 0.9rc1 marks the
> > pn_XXX_get_context()/pn_XXX_set_context() set of APIs as being deprecated.
> >
> > I use these apis fairly frequently as a means to map back to my
> > application's context.
> >
> > What are they being replaced with?   I couldn't find an associated JIRA
> > explaining how to upgrade my code.
> >
> > thanks,
> >
> 
> The context API has been replaced with the attachments API. The attachments
> API provides two key improvements over contexts. Firstly, you can have
> multiple attachments for a given object without them interfering with each
> other. Secondly, it permits you to define the type of each attachment,
> thereby allowing simpler options for memory management since you can use
> reference counted pointers if you wish.
> 
> The general pattern is:
> 
>     PN_HANDLE(FOO) // this is a macro that defines FOO to refer to a static
> memory location guaranteed to be unique within the program
> 
>     pn_record_t *record = pn_xxx_attachments(xxx);
>     pn_record_def(record, FOO, PN_VOID | PN_OBJECT);
>     pn_record_set(record, FOO, value);
>     void *value = pn_record_get(record, FOO);
> 
> For real use you'll probably want to wrap the get/set with accessors that
> cast foo to/from a more specific type, e.g.:
> 
>     pn_foo_t *pn_xxx_get_foo(xxx);
>     void pn_xxx_set_foo(xxx, pn_foo_t *foo);
> 
> I just noticed the pn_record_t API is missing doxygen. I'll add it to the
> list for a doc blitz. I have a plane ride coming up soon, so hopefully will
> have some time to churn out the missing doc pieces here.
> 
> --Rafael
> 

-- 
-K

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Re: proton application context API deprecated in 0.9?

Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Mon, Mar 9, 2015 at 10:35 AM, Ken Giusti <kg...@redhat.com> wrote:

> Hi,
>
> I just noticed that the 0.9rc1 marks the
> pn_XXX_get_context()/pn_XXX_set_context() set of APIs as being deprecated.
>
> I use these apis fairly frequently as a means to map back to my
> application's context.
>
> What are they being replaced with?   I couldn't find an associated JIRA
> explaining how to upgrade my code.
>
> thanks,
>

The context API has been replaced with the attachments API. The attachments
API provides two key improvements over contexts. Firstly, you can have
multiple attachments for a given object without them interfering with each
other. Secondly, it permits you to define the type of each attachment,
thereby allowing simpler options for memory management since you can use
reference counted pointers if you wish.

The general pattern is:

    PN_HANDLE(FOO) // this is a macro that defines FOO to refer to a static
memory location guaranteed to be unique within the program

    pn_record_t *record = pn_xxx_attachments(xxx);
    pn_record_def(record, FOO, PN_VOID | PN_OBJECT);
    pn_record_set(record, FOO, value);
    void *value = pn_record_get(record, FOO);

For real use you'll probably want to wrap the get/set with accessors that
cast foo to/from a more specific type, e.g.:

    pn_foo_t *pn_xxx_get_foo(xxx);
    void pn_xxx_set_foo(xxx, pn_foo_t *foo);

I just noticed the pn_record_t API is missing doxygen. I'll add it to the
list for a doc blitz. I have a plane ride coming up soon, so hopefully will
have some time to churn out the missing doc pieces here.

--Rafael