Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tuples and disposable objects #6620

Closed
lucamorelli opened this issue Mar 18, 2016 · 28 comments
Closed

tuples and disposable objects #6620

lucamorelli opened this issue Mar 18, 2016 · 28 comments
Assignees
Milestone

Comments

@lucamorelli
Copy link

Hi,
I'm following the discussion about tuples, and I have a question about this.
Often I have methods the return disposable objects (the most common is a ef or ado.net db connection, and to be sure that are correctly dispose I use using (...

using( var db = obj.getDbConnection()) {
 ...
}

Now, if one of the members of the tuple is an object like this, how can I handle its disposal in a proper way? For example I can have a method that returns both an ado.net sqlconnection and the related transaction object, because I want to encapsulate the creation procedure.

@HaloFour
Copy link
Contributor

The tuple itself wouldn't be disposable. As of now I doubt that the using statement would be allowed on a tuple type regardless of whether it has some elements that are disposable, but that could be subject to a proposal.

@gafter
Copy link
Member

gafter commented Mar 18, 2016

@MadsTorgersen @VSadov @jcouv FYI

@orthoxerox
Copy link

This is an interesting problem. At first glance, tuples should be IDisposable, since FDG say:

DO implement the Basic Dispose Pattern on types containing instances of disposable types.

However, tuples don't really contain instances of disposable types, just like List<T> doesn't and therefore doesn't implement IDisposable. If you get a collection of instances of disposable types, it's your job to extract them and dispose of them correctly.

But providing a convenient way to handle tuples with disposable content would simplify a lot of use cases.

Therefore I think let should be allowed inside using along with var:

using (let (status, bmp) = TryCaptureImage()) {
    ...
} ///bmp is disposed of

This way a tuple is deconstructed ASAP and you are dealing with raw values. If you really need to store the tuple, then you handle it manually, like you would handle a List<IDisposable>.

@bondsbw
Copy link

bondsbw commented Mar 20, 2016

Do the current tuple proposal/design also include indexed locations? We could do something like

using ((var (a, b, c) = ThisMethodReturnsATuple()).Item2) { ... }

// simplified if return value isn't used in the block
using (ThisMethodReturnsATuple().Item2) { ... }

or perhaps

using ((var (a, b, c) = ThisMethodReturnsATuple())[1]) { ... }

// simplified if return value isn't used in the block
using (ThisMethodReturnsATuple()[1]) { ... }

Downside, that is limited to a single item within the tuple.

@bondsbw
Copy link

bondsbw commented Mar 20, 2016

Possible new syntax which allows disposing of more than one item within the tuple:

using (a, b) in (var (a, b, c) = ThisMethodReturnsATuple()) 
{
    ... 
}
using b in (var (a, b, c) = ThisMethodReturnsATuple()) 
{
    ... 
}

@orthoxerox
Copy link

After sleeping on this, what's wrong with existing language support plus tuple destructuring?

let (status, bmp) = TryCaptureImage();
using (bmp) {
   ...
}

@alrz
Copy link
Contributor

alrz commented Mar 20, 2016

I think one should be able to implement IDisposable for tuples via dotnet/roslyn#8127:

extension<T, U> (T, U) : IDisposable where T : IDisposable where U : IDisposable {
  public void Dispose() {
    this.Item1.Dispose();
    this.Item2.Dispose();
  }
}

which is the same as defining an extension method (#258).

static class ValueTupleExtensions {
  public static Dispose<T, U>(this (T, U) @this) where T : IDisposable where U : IDisposable {
    @this.Item1.Dispose();
    @this.Item2.Dispose();
  }
}

@alrz
Copy link
Contributor

alrz commented Mar 20, 2016

Although, if just one of the items was IDisposable @orthoxerox's suggestion would be sufficient.

@bondsbw
Copy link

bondsbw commented Mar 20, 2016

@alrz It could be used for multiple as well, albeit a bit clunky:

let (status, bmp) = TryCaptureImage();
using (status)
using (bmp)
{
    ...
}

I would prefer a way to combine them in one line.

@VSadov
Copy link
Member

VSadov commented Mar 23, 2016

I think making tuples to implement IDisposable could be a very fragile approach, considering that they are structs. It would be too easy to dispose a copy.

What might be a more robust approach is teach "using" to decompose tuples into nested guarded blocks.

The issue with that approach would be finding a clever syntax for introducing tuple fields as standalone variables inside using.

using (a: Foo(), b: Bar())
{
     a.Do();
     b.Work();
}

emitted as

using (var a = Foo())
{
   using (var b = Bar())
   {
        a.Do();
        b.Work();
   }
}

@VSadov VSadov closed this as completed Mar 23, 2016
@VSadov VSadov reopened this Mar 23, 2016
@DerpMcDerp
Copy link

C# could be modified to allow the inline C++ style RAII syntax:

if (asdf) {
    something1();
    using T foo = whatever1();
    using bar = whatever2();
    something2();
    // bar gets disposed here
    // foo gets disposed here
}

in addition to the current lisp style nesting syntax:

if (asdf) {
    something1();
    using (T foo = whatever1()) {
        using (var bar = whatever2()) {
            something2();
            // bar gets disposed here
        }
        // foo gets disposed here
    }
}

If these changes were made then tuple element disposal falls out naturally with this syntax:

let (using a, using b) = Foo();

@paulomorgado
Copy link

Why go to all the trouble? Just deconstruct the tuple and use the items in any way you see fit. If it's using them on a using statement, be it.

@xied75
Copy link

xied75 commented Oct 13, 2016

Sorry to ask, given this got a milestone, what is the conclusion/action that is going to be implemented?

Since "using" is a compiler magic, and most of time people would call it like this:

using (var v = somemethodcall())
{
}

It is very reasonable to expect the same magic can dispose tuple returned by method call, it fits us average programmers' understanding of "using".

One of the reason could forcing us to use tuple e.g. Tuple<int, Stream> is that normally we could get the int by using out, but if the method call is async, then we are not allowed to have "out" there.

Having multiple lines to work around this just looks ugly when "using" is already a magic, half done magic is ugly.

@jcouv
Copy link
Member

jcouv commented Oct 14, 2016

@xied75 With our release candidate date closing in fast, this (or some flavor for it) isn't planned for C# 7.0.
Keeping the issue assigned to 2.0 is good to remind LDM to discuss it, but I don't want to give wrong expectations. Most likely we'll want to get feedback and see how people use tuples first, to help prioritize various features.

@StefanBertels
Copy link

Hi, is there any update on disposable tuples?

After reading the comments above (and after using tuples a lot in code) I think tuples should be disposable -- at least if a containing element is disposable.

One solution could be (I don't know whether compatible with CLR): If the tuple has one or more member type that implements IDisposable then the tuple itself should IDisposable (and forward dispose call to all members).

This would mean (edge case) that Tuple<object, Stream> is IDisposable and Stream will be disposed, object not. One could argue that disposing object via (obj as IDisposable)?.Dispose() might be a good idea or not (i.e. call Dispose() on runtime for values that are IDisposable regardless of the tuple part.
I think the answer might be "not" here because Tuple<object, object> wouldn't be IDisposable at all (at compile time) and this fits to current using (only possible on type implementing IDisposable).

Regarding comments that say one could easily workaround by explicitely calling Dispose on tuple parts or even use some extension method: This does not solve use cases where you pass the tuple to some code where it's relevant whether the type implements IDisposable. If you build generic code that creates objects and removes them later that code probably should check whether the created object is IDisposable because the generic code owns it and is responsible for the lifetime. If the object is a tuple containing some IDisposable you currently have a problem.

@orthoxerox pointed out that it all depends on whether Tuple contains the items. It's ok to see this both ways but I would argue that we should see what the pros and cons are.

If you see tuples as syntactic replacement for multiple single variables (grouping of variables) then it would be quite useful to have this IDisposable (at least if one or more items are...)

Tuples are most useful this way and C# is designed to support this by it's syntax. E.g. tuples are a great way to get rid of out parameters.

Currently I cannot use tuples for use cases above (generic code) but will have to create custom types. As ValueTuple<> is sealed I cannot easily build my own tuple type on top of it that at least would be IDisposable on runtime, either.

@CyrusNajmabadi
Copy link
Member

Hi, is there any update on disposable tuples?

Nope :)

Regarding comments that say one could easily workaround by explicitely calling Dispose on tuple parts or even use some extension method: This does not solve use cases where you pass the tuple to some code where it's relevant whether the type implements IDisposable. If you build generic code that creates objects and removes them later that code probably should check whether the created object is IDisposable because the generic code owns it and is responsible for the lifetime. If the object is a tuple containing some IDisposable you currently have a problem.

This is true of any code that checks for some interface, but you want to pass a tuple of values to.

In this case, tuples are not the right mechanism, and you should build your own type that can properly implement whatever interfaces are relevant to you, and forward them appropriately to any elements it holds.

As @orthoxerox said:

However, tuples don't really contain instances of disposable types, just like List doesn't and therefore doesn't implement IDisposable. If you get a collection of instances of disposable types, it's your job to extract them and dispose of them correctly.

@StefanBertels
Copy link

@CyrusNajmabadi: I don't agree since tuples have more in common with function arguments than with lists.

Currently you cannot have generic code that handles lifetimes based on IDisposable correct when using factory methods that have more then one return value -- without creating a custom class.

Some generic method:

TResult ReadRessource<T, TResult>(Func<T> creator, Func<T, TResult> reader) where T: IDisposable
{
  using(var res = creator())
  {
     return reader(res);
  }
}

You could call this:

var result = ReadRessource(
    () => File.OpenRead(filename), 
    stream => stream.ReadAllText());

which works fine now. T is just Stream here.

I would like to be able to call the same generic function with tuple this way, too:

var result = ReadRessource(
    () => (FileName: filename, Stream: File.OpenRead(filename)),
    tuple => tuple.FileName + ": " + tuple.Stream.ReadAllText());

This currently does not work because Tis (string, Stream) here which is not IDisposable.

BTW: In some future version of C# where #258 is reality this could be just:

var result = ReadRessource(
    () => (FileName: filename, Stream: File.OpenRead(filename)), 
    ((filename, stream)) => filename + ": " + stream.ReadAllText());

This is of course just a small example which can easily replaced by other code. It's just to show the principle.

@HaloFour
Copy link
Contributor

@StefanBertels

Even if C# were to treat tuples of IDisposable as themselves disposable that would not resolve your issue with generic type constraints as the runtime type will never be IDisposable itself.

@paulomorgado
Copy link

@StefanBertels, the purpose of tuples is to represent a list of values somewhat related and unrelated. They do not represent an entity.

You can look at tuples as something like this:

(bool success, int value) TryParse(string text);

success has it's own meaning regardless the value of value and value has it's own meaning regardless of the value of success.

If the semantics of the above method is the same as int.TryParse, that means that you should expect value to be default(int) when success is false but, after that, value will be 0 regardless of success or unsuccess and you don't carry success around to know the meaning of value.

If the set of values has a meaning of its own (like being disposable) than you should create a specific type for that.

@jcouv
Copy link
Member

jcouv commented Aug 20, 2018

We're currently prototype "pattern-based using statement" for C# 8.0, so that types that implement Dispose (even via extension methods) can be used in a using, even those that don't implement IDisposable.
I believe that resolves part of the problem.
See #1623 for more details.

Note a small subtlety: such Dispose methods on tuple types should dispose elements in reverse order to be consistent with the language.

@StefanBertels
Copy link

@HaloFour Can you give more details? I currently don't see a reason why there cannot be some DisposableValueTuple<> that works like ValueTuple<> -- with similar direct compiler support.

@paulomorgado: Your TryParse example doesn't show whether tuple disposability would be good/bad as neither bool nor int implement IDisposable / cannot be disposed at all.
Let's change the example to TryOpenConnection or TryOpenStream.

(BTW: Better return value would be something like Option here.)

We can always build specific types, but ValueTuple reduces boilerplate code and gives syntactic sugar for simple use cases.

@jcouv Thanks for the link. This will only help with using -- not with other ways to manage tuple lifetimes directly.

@dotnet dotnet locked and limited conversation to collaborators Oct 28, 2022
@CyrusNajmabadi CyrusNajmabadi converted this issue into discussion #6621 Oct 28, 2022
This issue is being transferred. Timeline may not be complete until it finishes.

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests