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

PS implementation of the C# Using statement #9886

Closed
Graham-Beer opened this issue Jun 13, 2019 · 21 comments
Closed

PS implementation of the C# Using statement #9886

Graham-Beer opened this issue Jun 13, 2019 · 21 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. Resolution-No Activity Issue has had no activity for 6 months or more WG-Language parser, language semantics

Comments

@Graham-Beer
Copy link

Summary of the new feature/enhancement

The C# language provides a convenient syntax that ensures the correct use of IDisposable objects. I believe this would be a great addition to the PowerShell language, as the 'Using statement' simplifies the code that you have to write to create a resource and then finally clean up the object.

Without the using statement you are required to ensure that Dispose() is called, then followed by the Close() method. By implementing the 'Using statement' you can assume that all kinds of streams are getting closed.

I would like to see the PowerShell language use a similar syntax to C#, which would be using (expression) statement.
PowerShell does make use of the $using variable, but I don't believe adding the 'using' keyword will cause any issues.

using ($read = [system.io.StreamReader]::new('C:\tmp\test.txt')) {
    $read.Read()
}
@Graham-Beer Graham-Beer added the Issue-Enhancement the issue is more of a feature request than a bug label Jun 13, 2019
@vexx32
Copy link
Collaborator

vexx32 commented Jun 13, 2019

Come to think of it, I think this has come up in discussions previously, where the idea that this could actually be a cmdlet as well surfaced. Compiled cmdlets already have the capability to properly implement disposable features, after all.

Do you think a cmdlet would be better than a keyword? It might be possible to have it be pipeline-capable if it's a cmdlet, which could be interesting, depending on implementation, and cmdlets are a little more flexible than keywords...

Also, if it's a cmdlet we could package it in a separate module and put it on the PSGallery, too, so even PS5.1 could make use of it as well. 🤔

@Graham-Beer
Copy link
Author

An interesting theory creating as a cmdlet. I'm still feeling its nicer if a keyword then it would be part of the language and get utilized more.

@vexx32
Copy link
Collaborator

vexx32 commented Jun 13, 2019

I think that makes sense. We do have the unfortunate consequence there of it being unavailable on downlevel powershell versions, though.

Also, keywords tend to be a bit less discoverable than cmdlets in general; keywords don't tab-complete, but functions and cmdlets do, and they're a bit easier to find in the help documents as well -- most language features are documented in a slew of about_* help topics and there can be some topics that cover multiple things, whereas each cmdlet gets its own discrete topic.

I'm not against a keyword, despite all that. Keywords have a really nice no-nonsense simplicity about them that can be really nice to work with. Cmdlets can tend towards feature creep, among other things, and tend to be a bit slower to operate.

I'm not really sure which approach is really better. Either way, though, we should have this implemented in some form or another. 😄

@Graham-Beer
Copy link
Author

I think there is a good argument for both to be fair. I do get the backward compatibility but maybe it also encourages the use of PS 7 with shiny new features! Interesting to hear other peoples thoughts on this as well.

@SeeminglyScience
Copy link
Collaborator

SeeminglyScience commented Jun 13, 2019

It would be difficult to do using as a cmdlet without losing some safety. There's many situations where a pipeline stop exception could be thrown before the cmdlet received the IDisposable.

Last time this came up, @BrucePay suggested this syntax:

Use-Object { [SomeDisposable]::new() } {
    $PSItem.DoSomethingWithDisposable()
}

That could work, but I'd still be worried that a pipeline stop would be triggered before the cmdlet finished invoking the script block (that's assuming that MshCommandRuntime.WriteObject checks for pipeline stops, which I think it does). If it was a keyword then the compiler could generate a safer expression. It also just doesn't look as nice imo for whatever that's worth.

Also, keywords tend to be a bit less discoverable than cmdlets in general

eh... if you know enough to know that something needs to be disposed, you probably know about using.

@BrucePay
Copy link
Collaborator

@Graham-Beer The using discussion comes every couple of years and then fades away. I suspect this is due to the fact that there really haven't a been lot of scenarios where you need it in PowerShell and the few where it's required can be handled with try/finally. Otherwise the pipeline/cmdlets take care of disposing the resources for you. As a consequence, it has been hard to sustain much energy around this issue. I'm not against the idea (in fact I was quite keen on it at one point) but currently it's just kind of meh. So if you have some new scenarios where using is critical, please share them. Thanks!

@rkeithhill
Copy link
Collaborator

just kind of meh

That sums it up nicely for me as well.

@markekraus
Copy link
Contributor

Does a finally block run in a cmdlet/function even in the case of a pipeline stop exception?

As for the lack of sustained interest... I think the lack of decent using solution makes streams in PS awkward and discourages their use. I'm a HUGE fan of streams and have always felt they are a missed opportunity in PS. I have used them heavily in PHP, C#, and Python. Because they are clunky in PS, I will either Add-Type some C# in or write a library in C# to handle it for me. In a pinch I can use a try/finally, but it starts to get unruly and painful to read when you have multiple IDisposables to work with at once.

here's an example from an earlier issue:

function Get-RemoteCertificate {
  [CmdletBinding()]
  [OutputType([System.Security.Cryptography.X509Certificates.X509Certificate])]
  param (
    [Parameter(
      Mandatory,
      ValueFromPipeline
    )]
    [ValidateNotNull()]
    [Uri]
    $Uri
  )
  process {
    try {
      $TcpClient = [System.Net.Sockets.TcpClient]::new($Uri.Host, $Uri.Port)
      try {
        $SslStream = [System.Net.Security.SslStream]::new($TcpClient.GetStream())
        $SslStream.AuthenticateAsClient($Uri.Host)
        $SslStream.RemoteCertificate
      } finally {
        $SslStream.Dispose()
      }
    } finally {
      $TcpClient.Dispose()
    }
  }
}

@SeeminglyScience
Copy link
Collaborator

SeeminglyScience commented Jun 13, 2019

@markekraus

Does a finally block run in a cmdlet/function even in the case of a pipeline stop exception?

Yes, with some caveats. If you place the assignment inside the try, there could be an exception or stop prior to the assignment. If this happens in the try, the finally could still fire, potentially calling Dispose on an object from a previous scope. If you place the assignment outside the try, a stop could occur before getting to the try block.

So I've taken to a pattern like this:

$client = $null
try {
    $client = [TcpClient]::new($uri.Host, $uri.Port)
} finally {
    if ($null -ne $client) {
        $client.Dispose()
    }
}

It's not pretty, but it's safe.

@markekraus
Copy link
Contributor

oof... that's even more painful.

@vexx32
Copy link
Collaborator

vexx32 commented Jun 14, 2019

I have a PR in the works (#9900) to make this much easier for pipeline cmdlets (esp. those that need to keep the object around for the entire process{} sequence and only dispose it at the very end), but that scoping issue may well still be a bit of a problem. Not sure there's a neat solution to that one. 🤔

That's honestly one pretty clear case for a using keyword, I must say...

@rkeithhill
Copy link
Collaborator

If only we had ?. then your finally statement becomes a bit more simpler $client?.Dispose(). Man, love me some C#. :-)

@Graham-Beer
Copy link
Author

I quite like the C# syntax of ?, @rkeithhill. How do we push this topic idea forward?

@vexx32
Copy link
Collaborator

vexx32 commented Jun 20, 2019

I think there may be an existing issue for it, but if not create one -- if there already is one, add a comment.

@Cirzen
Copy link

Cirzen commented Jul 8, 2019

If there is an open issue for the ?. / safe navigation operator, could you link it here please? I would very much like to upvote!

@SeeminglyScience
Copy link
Collaborator

@Cirzen #3240 includes it.

@simonsabin
Copy link

Any time one ends up in the world of .net objects not having clean disposal is a pain. This is especially bad for file manipulation when file handles don't get disposed of. I needed to get the file entries in a zip file and resorting to system.io.compression and streams results in lots of ugly try catch finally and disposing

@SeeminglyScience SeeminglyScience added WG-Language parser, language semantics Needs-Triage The issue is new and needs to be triaged by a work group. labels Apr 12, 2022
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

2 similar comments
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-No Activity Issue has had no activity for 6 months or more labels Nov 16, 2023
Copy link
Contributor

This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. Resolution-No Activity Issue has had no activity for 6 months or more WG-Language parser, language semantics
Projects
None yet
Development

No branches or pull requests

8 participants