C#•3w ago

IDisposable ownership semantics

This is primarily a question regarding whether or not C# has introduced any sort of syntactic sugar or support for a "nicer" way of expressing transfer of ownership. For the purposes of this question, nullable types is fully enabled and static analysis that will issue CA2000 as an error is also turned on. Consider a method that returns a type that implements IDisposable and needs to either return it, or ensure it gets Dispose'd if an exception occurs - Currently I'd write something like this:

public SomeDisposable getDisposable() {
  var result = new SomeDisposable();
  try {
    //do possibly exception-throwing stuff here
    var ret = result;
    result = null;
    return ret;
  } finally {
    result?.Dispose();
  }
}

public SomeDisposable getDisposable() {
  var result = new SomeDisposable();
  try {
    //do possibly exception-throwing stuff here
    var ret = result;
    result = null;
    return ret;
  } finally {
    result?.Dispose();
  }
}

Is there anything better than this? This sort of pattern feels about as manual as malloc and free in C.

14 Replies

Kiel•3w ago

If you are using only try { } finally { }, perhaps consider using a, well...using statement or expression?

public SomeDisposable GetDisposable()
{
    using var result = new SomeDisposable();
    var ret = result.PossiblyThrows();
    return ret;
}

public SomeDisposable GetDisposable()
{
    using var result = new SomeDisposable();
    var ret = result.PossiblyThrows();
    return ret;
}

I'm not sure what the point of setting result to null is in your original code, but what I just posted above is syntactically equivalent to

public SomeDisposable GetDisposable()
{
  var result = new SomeDisposable();
  try
  {
      var ret = result.PossiblyThrows();
      return ret;
  }
  finally
  {
      result.Dispose();
  }
}

public SomeDisposable GetDisposable()
{
  var result = new SomeDisposable();
  try
  {
      var ret = result.PossiblyThrows();
      return ret;
  }
  finally
  {
      result.Dispose();
  }
}

OliproOP•3w ago

using is invalid because it would result in calling Dispose on the object it's returning.

Kiel•3w ago

if you prefer nesting and/or your true method is larger than this, you can also use a using expression instead:

using (var result = new SomeDisposable())
{
    var ret = result.PossiblyThrows();
    return ret;
}

using (var result = new SomeDisposable())
{
    var ret = result.PossiblyThrows();
    return ret;
}

So you only want result disposed if an exception is thrown?

OliproOP•3w ago

we are returning a type that implements IDisposable, if the method causes Dispose() to be invoked on the object, it's now returning an unusable instance.

Kiel•3w ago

var result = new SomeDisposable();

try
{
    var ret = result.PossiblyThrows();
    return ret;
}
catch
{
    result.Dispose();
    return // whatever you want
}

var result = new SomeDisposable();

try
{
    var ret = result.PossiblyThrows();
    return ret;
}
catch
{
    result.Dispose();
    return // whatever you want
}

Is this closer to what you're looking for? You could use try/catch instead of try/finally but keep in mind this approach would swallow exceptions, you should still probably log or whatever your system does to handle them if I'm reading your messages right you want result disposed ONLY if an exception is thrown otherwise, return it your original example was a little weird re-assigning result to a new variable so it threw me off

OliproOP•3w ago

the method has no obligation nor desire to catch exceptions. This is doing what finally would do, except we've now made it more long-winded because we'd have to rethrow, for no reason

Kiel•3w ago

what exactly are you trying to accomplish here then, if not what I'm suggesting?

OliproOP•3w ago

This is primarily a question regarding whether or not C# has introduced any sort of syntactic sugar or support for a "nicer" way of expressing transfer of ownership.

Kiel•3w ago

to answer your question bluntly, no C# does not have first class support for...whatever it is you're trying to express. We have try/finally, try/catch, and the various forms of using. But it seems like you're pretty much asking for a way to create an object, do work with it, and return it, and dispose it ONLY if an exception is thrown. If you're re-using this logic lots, you could probably use the above suggestion as a wrapper, but otherwise yeah the way you're doing it is probably about as clean as it's gonna get without re-throwing

OliproOP•3w ago

What I'm trying to express is code that complies with CA2000 - which requires that all IDisposable types aren't leaked. a lot of code I've seen that uses IDisposable types seems to essentially ignore the problem and hopes that the type has a finalizer that'll do the job when the GC gets around to collecting it

Anton•3w ago

You're doing it correctly, that's how I do it too I think that's the right way

OliproOP•3w ago

I know it's correct, I was hoping that in 2024 we don't still have to do this C-like crap. But I appreciate the validation 🙂 for reference, I'm cleaning up a bunch of code that currently leaks Bitmap all over the place

Anton•3w ago

I mean yeah it's easy to miss one without any help from the compiler You'd want defer and errdefer ideally errdefer for exceptions

OliproOP•3w ago

the compounding issue is that if a Bitmap is constructed from a Stream impl, you can't Dispose() the stream until after the Bitmap is disposed this could be simplified by immediately constructing a new Bitmap from the one that has the Stream so that the originals can then be Dispose'd but that'll absolutely massacre performance. This is irrelevant to the question. IDisposable is not supposed to be left to the GC to deal with. What you're saying about the GC is also not quite accurate either since .NET implements a generational garbage collector, it's not simply mark & sweep

Gaming

Programming

IDisposable ownership semantics