[Xorp-hackers] The XrlPFSender race

[Bruce Simpson]

Bruce Simpson wrote:
> I'm running out the door and don't really have time to finish this patch.
> But this is along the lines of what I'm really looking for.

That obviously didn't compile. There are some things to be aware of -- 
whilst you can test the value of a Boost smart pointer, and use it where 
a pointer would be used, direct C-style comparisons sometimes need 
explicit casting, or just use foo.get().

shared_ptr is used here only to manage the lifecycle of XrlPFSender 
within XrlRouter, which owns the object. class Xrl had no business 
holding an unchecked pointer to a shared object as a cache. weak_ptr 
allows class Xrl's cached reference to be validated upfront, and makes 
the intent behind it clearer in the source.

Attached is a patch which compiles, and passes a 'scons check'. If you 
guys could test further, that would be great.

Points about using shared_ptr<T>/weak_ptr<T> vs ref_ptr<T>:
 * Implementation is Boost community supported and mature.
 * Implementation follows pimp/rimpl.
 * Semantics are more explicit.
  * weak_ptr observes a resource.
  * shared_ptr owns a resource.
 * Will use pthread locks to acquire resource if threaded;
   if not, atomic machine-specific instructions will be used;
   finally, portable spin-locks are used.

For now, Boost thread support is explicitly disabled.

However, the counter pool used by shared_ptr<T> is private to its 
instantiation.  A shared base class is used, but unlike ref_ptr<T>, we 
don't explicitly instantiate it within shared code, so this leads to a 
little template instantiation in libxorp_ipc.so for the time being.

I measure the binary size gain in libxorp_ipc.so on FreeBSD 8.0/amd64 (I 
have now updated):
 +8270 bytes text
 +296 bytes data

I can live with that, as the race is fairly deep in the core.

No performance profiling of this change. A basic router appears to run 
fine with this change.

General coding advice:

 * Hold shared_ptr<T> across operations which require it.
 * Use weak_ptr<T> in situations where the pointer is being cached or
   crossing subsystem boundaries, or where it goes to existing,
   non-threaded XORP APIs, e.g. callbacks.
 * No need for enable_shared_from_this<T> yet, we aren't going to
   push the weak_ptr further down.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: xrlpfsender-sharedptr-cut2.diff
Url: http://mailman.ICSI.Berkeley.EDU/pipermail/xorp-hackers/attachments/20091217/12ebb5e2/attachment-0001.ksh