Skip to content
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 #9882

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

tuples and disposable objects #9882

lucamorelli opened this issue Mar 18, 2016 · 30 comments

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

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
Contributor

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
Contributor

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 #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.

@jaredpar jaredpar modified the milestone: 2.0 (Preview 3) Apr 25, 2016
@gafter gafter modified the milestones: 2.0 (Preview 3), 2.0 (RC) Jul 14, 2016
@jaredpar jaredpar modified the milestones: 2.0 (RC), 2.0 (RTM) Jul 18, 2016
@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.

@jaredpar jaredpar modified the milestones: 2.0 (RTM), Unknown Mar 27, 2017
@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 dotnet/csharplang#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

@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 dotnet/csharplang#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.

@HaloFour
Copy link

@StefanBertels

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.

Because such a type won't work. Either all of the generic type arguments would have to be disposable, or none of them could be. Your example of having one element that is disposable and another that isn't wouldn't be possible to model or support via generics.

@paulomorgado
Copy link

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

@StefanBertels, you definitely don't understand what tuples are or are forcing them to be what they aren't.
Your point about an Option type reinforces that.

An Option<int> type represents something entirely different. It represents something that might or might not be an int whereas (bool success, int value) returned by TryParse(string text) represents two different things: 1) whether the parsing operation was successful or not and the produced value. Exactly the same thing as if one (or both) of them was an out parameter.

@bondsbw
Copy link

bondsbw commented Aug 21, 2018

@paulomorgado Are you talking about the semantic meaning of Option<int>? I don't think that's relevant at this level of discussion.

The expressiveness of something like Option<int> (or int? or call it what you want) is the better fit since it can represent exactly these ideas:

  • Every 32-bit integer
  • An indication that the value is outside of the 32-bit integer space

As opposed to (bool, int) which can represent these ideas:

  • Every 32-bit integer (true)
  • Every 32-bit integer again (false)

Really this is off topic unless I am misinterpreting the intention of your comment.

@StefanBertels
Copy link

@paulomorgado I don't think you understood me correctly.
I didn't want to confuse anyone here by adding my Option<int> comment. This was just some note regarding semantics of your special example. This isn't related to tuples at all. IMO one should look into this before designing function signatures for tasks like TryGet() / TryParse(). YMMV.

Technically there is nothing wrong with your example but to see effects for Disposables the tuple should at least have one Diposable in there.

@HaloFour: Why do you think this wouldn't be possible?
There is no need for tuples to consist of identical or compatible types. The compiler knows whether individual member types are IDisposable or not.
Think about class DisposableValueTuple1<T1, T2> : IDisposable where T1: Disposable

I'm sure there are some technical issues with disposable tuples -- like compatibility. But I don't see it cannot be done at all. Don't get me wrong: I'm not sure whether Disposable tuples would make things better or worse, probably both :-). Therefore I'm interested in use cases where something like this would mislead developers / create lifetime issues.


Question: Does anybody think, tuple members should not be IDisposable at all (=> code smell => analyzers should warn)?

@HaloFour
Copy link

@StefanBertels

Think about class DisposableValueTuple1<T1, T2> : IDisposable where T1: Disposable

  1. tuples aren't classes, they're structs
  2. In that case T2 can't be disposable. You'd need yet another type to make that possible. And a third type if T2 is disposable but T1 isn't. This leads to type explosion, and since each type is a struct they are incompatible with one another.

At best you can hope for special compiler support if the compiler knows that the generic type arguments are disposable, but that won't work in the generic cases since the compiler will not know if and how those tuples are disposable.

@paulomorgado
Copy link

Exactly my point, @bartonjs. Tuples are just bags of values with no common or aggregated meaning.

@bondsbw
Copy link

bondsbw commented Aug 21, 2018

@paulomorgado That's what I don't understand about your comment. The result of a TryXXX() method should have an aggregated meaning.

In (bool success, int value) you should not assume that value has any useful meaning without the context provided by success. A tuple is not a good fit for providing holistic meaning (I would make the same argument against the existing combination of bool return and out parameter).

@CyrusNajmabadi
Copy link
Member

Closing this out. We're doing all language design now at dotnet/csharplang. If you're still interested in this idea let us know and we can migrate this over to a discussion in that repo. Thanks!

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2022
@StefanBertels
Copy link

I still think this would be a valuable thing to have.

Maybe a void Dispose(this (T1, T2) tuple) extension method could be a working solution (duck typing)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests