[Pvfs2-developers] BMI_testunexpected and free

[Phil Carns]

Pete Wyckoff wrote:
> slang at mcs.anl.gov wrote on Fri, 18 Aug 2006 21:29 -0500:
> 
>>Thanks for the method details.  My concern is mostly with the  
>>interface semantics for future bmi method implementations, as well as  
>>other users of bmi (possibly outside of pvfs).  Could a BMI method  
>>potentially return unexpected message buffers not allocated with malloc?
> 
> 
> You mean return buffers that were memalloc()-ed to work with the
> network?  Or return static arrays?  I guess those would both be
> possible.  It is quite natural in a BMI implementation just to alloc
> a bunch of small unexpected receive buffers once and keep reusing
> them.
> 
> No matter what, all BMI implementations had better be consistent.
> 
> If you have a particular use case in mind, maybe it'll help me to
> understand why you're pondering this issue.
> 
> 
>>Also, from a consistency perspective, I would prefer to have bmi  
>>methods return malloced buffers, and not expect the user to know to  
>>call free on that buffer.  If only that bmi may at some point be a  
>>separate library, allocating memory in one library and freeing it in  
>>another is usually considered bad practice.
> 
> 
> How does BMI know when the user is done with the buffer?  Somebody
> has to say the memory can be used for other purposes sometime, else
> the unexepected buffers just pile up.
> 
> You could add a separate API call BMI_testunexpected_ack_buffer().
> Or you could also force the user to call BMI_post_unexpected() to
> pass in its own buffers.  Or change testunexpected() to take buffers
> that would be filled in by BMI if a message appeared.
> 
> Maybe you've seen the clumsy API where you first pass in a NULL
> buffer and an int*.  The function tells you how big your buffer
> needs to be via the int*.  Then you alloc it, and call again, this
> time with the right-sized buffer.  Crazy, that, but it keeps the
> allocation and free in the same place.  Can't think of anything in
> libc that does it, thank goodness, but the old IB library used that
> often.
> 
> With a separate malloc(), lifetime rules and ownership are well
> defined: once returned, the buffer is the problem of the caller.  I
> agree it's a bit of a thing to force people to think about, but
> plenty of examples exist:  asprintf, getaddrinfo, glob, regcomp,
> realpath, tempnam, ... .  It would be nice to document well this
> behavior in BMI.

For my 2c, I would prefer one of these approaches:

- the current mechanism (make user call free())

- add a BMI function specifically for freeing unexpected buffers 
(BMI_unexpected_free() or something like that).

The latter approach would just be implemented by calling free() for the 
current methods, and future methods would have the freedom to do 
whatever they please.  I would rather not overload the BMI_memfree() 
function since it is meant to be paired with BMI_memalloc() and would 
imply that we are dictating that the unexpected buffer be optimized for 
communication.  I also would also rather not add any extra arguments or 
function calls to specify the unexpected buffer in advance- as Pete 
points out that can get icky in a hurry.

As far as the lifetime rule for the unexpected buffer... the only reason 
we held onto it that long was to support the possibility of having 
encoding mechanisms that decode "in place".  If someone were to 
implement that, then of course the unexpected buffer would need to hang 
around until after PINT_decode_release() is called.  Personally, I'm not 
particularly interested in that type of en/decoding because it is hard 
to make something like that work in heterogeneous environments.  My vote 
is to go ahead and destroy the unexpected buffer sooner if possible.  If 
anyone ever wants to go through the pain of implementing an in-place 
decoder then they can move the free back to a later point again (it 
isn't a difficult change).

-Phil