views:

50

answers:

1

This is a question regarding coding design, so please forgive the long code listings: I could not resume these ideas and the potential pitfalls without showing the actual code.

I am writing a ConcurrentReferenceCounted class and would appreciate some feedback on my implementation. Sub-classes from this class will receive "release" instead of a direct delete.

Here is the class:

class ConcurrentReferenceCounted : private NonCopyable {
 public:
  ConcurrentReferenceCounted() : ref_count_(1) {}

  virtual ~ConcurrentReferenceCounted() {}

  void retain() {
    ScopedLock lock(mutex_);
    ++ref_count_;
  }

  void release() {
    bool should_die = false;
    {
      ScopedLock lock(mutex_);
      should_die = --ref_count_ == 0;
    }
    if (should_die) delete this;
  }

 private:
  size_t ref_count_;
  Mutex mutex_;
};

And here is a scoped retain:

class ScopedRetain {
public:
  ScopedRetain(ConcurrentReferenceCounted *object) : object_(object) {
    retain();
  }

  ScopedRetain() : object_(NULL) {}

  ~ScopedRetain() {
    release();
  }

  void hold(ConcurrentReferenceCounted *object) {
    assert(!object_); // cannot hold more then 1 object
    object_ = object;
    retain();
  }

private:
  ConcurrentReferenceCounted *object_;

  void release() {
    if (object_) object_->release();
  }

  void retain() {
    object_->retain();
  }
};

And finally this is a use case:

Object *target;
ScopedRetain sr;

if (objects_.get(key, &target))
  sr.hold(target);
else
  return;

// use target
// no need to 'release'
+1  A: 

Your ConcurrentReferenceCounted seems to use a full mutex, which is not necessary and not very fast. Reference counting can be implemented atomically using architecture-dependent interlocked instructions. Under Windows, the InterlockedXXXfamily of functions simply wraps these instructions.

Alexander Gessler
I am writing portable code across Windows, Linux, Embedded linux and Mac OS X: I am not sure I can find interlocked equivalents for all these platforms...
Gaspard Bucher
Keep looking, big difference. Or you'll end up with a "sucks everywhere" product.
Hans Passant
The linux kernel defines a `atomic_t` data type, which has assembly implementations for many platforms. It's up to you and your performance requirements whether it's worth the effort or not.
Alexander Gessler
Nobody wants to write software that "sucks everywhere" ;-) so I will use an atomic increment, but does this work for "decrement and compare" ?
Gaspard Bucher
If you want an example of a lock-free concurrent counter that works under all the OS's you mentioned, check out the code at the following link. It's BSD licensed so you can use it however you like: https://public.msli.com/lcs/muscle/muscle/system/AtomicCounter.h
Jeremy Friesner
Yes, it works - you do `--ref_cnt` using an exchange-and-add operation, which gives you the original value of the reference counter before the decrement. If multiple threads do it concurrently, only one thread gets a previous value '1' in return.
Alexander Gessler
@Jeremy: Thanks for the link ! Looks great, much more manageable the openpa. I'll try to include it in http://rubyk.org/oscit and will let you know how it goes.
Gaspard Bucher