C# version of C++ shared_ptr

Question

I need something akin to C++'s shared_ptr. Essentially I need an IDisposable item that I can ensure is cleaned up as soon as possible once all references have been removed, but this resource will be shared between many classes.

I don't allow passing in of a an already existing object as it's an easy way to know that the item I'm using isn't being hasn't already been disposed.

Unfortunately I can't think of a way to prevent the user of Item from disposing the underlying item, but as this is to go into my own program I'm not too concerned about that because it intuitively makes sense to not dispose something within a wrapper, provided every class that needs the item uses the wrapper and not just the contained item.

I simply want to know if anyone can see any potential issues with this solution besides what's mentioned.

public class SharedDisposable<T> : IDisposable where T : class, IDisposable {
    static SharedDisposable(){
        instances = new Dictionary<T, List<SharedDisposable<T>>>();
    }

    public SharedDisposable(SharedDisposable<T> share) {
        Item = share.Item;
        instances[Item].Add(this);
    }
    public SharedDisposable(object[] ctorArgs = null) {
        if(ctorArgs == null)
            ctorArgs = new object[] { };

        var ctor = typeof(SharedDisposable<T>).GetConstructor(ctorArgs.Select(a => a.GetType()).ToArray());
        Item = (T)ctor.Invoke(ctorArgs);
        instances[Item] = new List<SharedDisposable<T>>() { this };
    }
    public SharedDisposable(KeyValuePair<Type, object>[] ctorArgs) {
        var ctor = typeof(SharedDisposable<T>).GetConstructor(ctorArgs.Select(a => a.Key).ToArray());
        Item = (T)ctor.Invoke(ctorArgs.Select(a => a.Value).ToArray());
        instances[Item] = new List<SharedDisposable<T>>() { this };
    }

    public void Dispose() {
        var instanceList = instances[Item];
        instanceList.Remove(this);
        if(instances.Count == 0) {
            instances.Remove(Item);
            Item.Dispose();
        }
    }

    public T Item { get; }
    static Dictionary<T, List<SharedDisposable<T>>> instances;
}

Example usage:

class MainForm : Form {
    public MainForm(){
        InitializeComponent();

        Port = new SharedDisposable<SerialPort>("COM5", 9600);
    }
    public MainForm(SharedDisposable<SerialPort> share){
        InitializeComponent();

        if(share == null)
            throw new ArgumentNullException(nameof(share))
        Port = new SharedDisposable<SerialPort>(share);
    }

    public void BtnDevice1Info_Click(object sender, EventArgs e){
        new Device1InfoForm(Port).Show();
    }
    public void BtnDevice2Info_Click(object sender, EventArgs e){
        new Device2InfoForm(Port).Show();
    }

    override Dispose(bool disposing){
        if(disposing){
            Port.Dispose();
        }
        base.Dispose(disposing);
    }

    private SharedDisposable<SerialPort> Port { get; }
}

In my application I actually have a Modbus interface instead of serial port but I just realised the way the constructor is setup won't allow me to use the Modbus API because the Modbus API uses factories instead of constructors, so I'll need to adjust it for that. But that API should be able to queue messages to be sent out one at a time, preventing any conflicts on the serial port while also allowing multiple forms or even applications to use the same serial port. In order to make this happen I need multiple forms to be able to share a disposable resource, but none of them to clean it up unless they're the last holder of that resource.

Answer 1

Adding to t3chb0t's answer...

Thread safety

The current solution isn't thread safe, as access to instances isn't synchronized. Maybe use a lock or a thread-safe dictionary class instead?

In C++, std::shared_ptr ensures that updates to the control block ("ref count") are thread safe (atomic). This means creating copies or destroying an instance of a std::shared_ptr doesn't corrupt the control block, i.e. can safely be made in a concurrent environment.

Memory usage

It isn't really necessay to store a List<SharedDisposable<T>> in instances: A simple counter would be enough. This reduces the memory footprint by quite a lot.

Design

Is instances as a static field really necessary? There are other design possibilities that don't have to rely on a static field, and are more extensible if you later realize that you need a std::weak_ptr equivalent.

For example, the counter (and surrounding logic) could be kept inside a ControlBlock (to use the C++ standardese name) class. Then, an instance of ControlBlock gets created with the first SharedDisposable reference, and every subsequent copy gets a reference to it. No need to manage pseudo-global state!

However, this would eliminate the (easy) possibility of iterating over all SharedDisposable<T> for a given T.

Answer 2

public SharedDisposable(object[] ctorArgs = null)
{
  if (ctorArgs == null)
      ctorArgs = new object[] { };

  var ctor = typeof(SharedDisposable<T>).GetConstructor(ctorArgs.Select(a => a.GetType()).ToArray());
  Item = (T)ctor.Invoke(ctorArgs);
  instances[Item] = new List<SharedDisposable<T>>() { this };
}

This constructor is not a very good idea because the use needs to exactly know the signature of the constructor in order to create T.

A much better way would be to let him specify a factory lambda. Then all it will be left is:

public SharedDisposable(Func<T> create)
{
    Item = create();
    instances[Item] = new List<SharedDisposable<T>>() { this };
}

so now he can call it in a much easier fashion like:

Port = new SharedDisposable<SerialPort>(() => new SerialPort("COM5", 9600));

The other constructor using KeyValuePairs is now obsolete.

---

public SharedDisposable(object[] ctorArgs = null)
{
  if (ctorArgs == null)
      ctorArgs = new object[] { };
  ...
}

Btw. you can make this look cleaner with the coalesce operator: ??

ctorArgs = ctorArgs ?? new object[] { };

---

Objects like this one usully use the name Value for what you chose Item.

---

Its usage might be easier in certain scenarios if you added an implicit cast operator to T:

public static implicit operator T(SharedDisposable<T> share) => share.Value;

---

If you replaced the List by a HashSet the Dispose method could be simplified to:

public void Dispose()
{
    if (instances[Value].Remove(this) && instances[Value].Count == 0)
    {
        instances.Remove(Value);
        Value.Dispose();
    }
}

Answer 3

Revision 2

Taking into account all your feedback, this is what I have now.

• Replaced the constructors with a single constructor that accepts a function for creating the instance. This will allow constructor or factories to be used, and also avoids any issues with type-mismatching in Reflection.
• Added locks to everywhere I believe is necessary to provide thread safe functionality.
• Changed the static dictionary of T and instances, for a class containing a the value and a counter, which will be referenced from shared copies.

• Added a .Share() function as a synonym for the constructor, to be used in using.

  public class SharedDisposable<T> : IDisposable where T : class, IDisposable {
      private class Target {
          public T value;
          public uint count;
      }
  
      // Lambda constructor, because reflection is too strict.
      // This will also help with classes that use factories.
      public SharedDisposable(Func<T> create) {
          target = new Target() {
              value = create(),
              count = 1,
          };
      }
  
      public SharedDisposable(SharedDisposable<T> share) {
          lock(share.target) {
              // Check that Dispose() did not get a lock and dispose the
              //   object before we are able to obtain a reference to it.
              if(target.count == 0)
                  throw new ObjectDisposedException(nameof(share), "Target object has been disposed before being shared.");
              target = share.target;
              target.count++;
          }
      }
  
      // To be used as a temporary lock, such as in `using()`.
      // Can also be used as a synonym for copy c'tor.
      public SharedDisposable<T> Share() {
          return new SharedDisposable<T>(this);
      }
  
      public void Dispose() {
          lock(target) {
              if(--target.count == 0) {
                  //We don't care if Dispose takes a while as we're the only lock
                  //  holder of `target` so the lock isn't blocking any other threads
                  //  Unless there's some time related overhead of `lock()` ?
                  Value.Dispose();
              }
          }
      }
  
      private Target target = null;
      public T Value { get => target.value; }
  }
  

Example usage of using()

// Prevent the object from being disposed while being worked on
//   by creating a new share in a using() statement.

using(var lockShare = mySharedObj.Share()){
    //do stuff on lockShare
}