-
Notifications
You must be signed in to change notification settings - Fork 133
Automatic resource management #250
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| --- | ||
| RFC: RFC<four digit unique incrementing number assigned by Committee, this shall be left blank by the author> | ||
| Author: Ilya Sazonov | ||
| Status: Draft | ||
| SupercededBy: <link to another RFC> | ||
| Version: 1.0 | ||
| Area: Engine | ||
| Comments Due: 7/1/2020 | ||
| Plan to implement: Maybe | ||
| --- | ||
|
|
||
| # Automatic resource management | ||
|
|
||
| ## Rationale | ||
|
|
||
| Currently PowerShell has limited capabilities compared with C# to excecute a mandatory code. | ||
|
|
||
| PowerShell users can benefit from `finally` block of `try - catch` statement. It is very limited experience because resources in PowerShell can be distributed across scopes, cmdlets, pipelines and runspaces. | ||
|
|
||
| If an user created a temporary file one should worry about deleting it, while PowerShell could register a mandatory code to delete this file and execute it automatically at the right time. | ||
|
|
||
| PowerShell makes many smart things for users and could offer smart resource management. | ||
|
|
||
| ## Motivation | ||
|
|
||
| As a script writer, I can use some well-known API-s which grabs resources | ||
| and I want PowerShell understands this and releases the resources automatically at the right time | ||
| so that avoids resorce leaks. | ||
|
|
||
| As an advanced module and script writer, I can use some API-s which grabs resources | ||
| and I want to register a code PowerShell must run to release the resources automatically at the right time so that avoids resorce leaks. | ||
|
|
||
| As an advanced module and script writer, I want to make sure that a mandatory (cleanup, logging) code is always executed even if a pipeline / script / cmdlet is unexpectedly interrupted. | ||
|
Comment on lines
+30
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section sounds very much like #207, I'm not clear on what this would offer that's not already captured in that RFC.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's also significantly less versatile, and doesn't allow authors to really register their own code to be handled during disposal, but instead tries to do "some things" for them, in a way that's less controllable and less visible in terms of when that cleanup/disposal happens.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vexx32 All your comment comes from #207 RFC position. It does not add value in the discussion. Current RFC is about another concept. If you want to discuss the concept you should forget that RFC and freely play with the concept.
No, developer can register any complex code like in C#. Rather, we should even limit it so that this code really does the cleanup and nothing more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood. I don't know how you'd go about limiting that in any appreciable manner, though. |
||
|
|
||
| As binary module developer I want to benefit from this in binary modules too. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Binary cmdlets already can implement
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This says that modules could issue annotated objects, and more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does that add value for cmdlet/compiled module authors? |
||
|
|
||
| ## User Experience | ||
|
|
||
| User has no needs explicitly dispose an object when out of the script block: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This basically sounds like RAII or the way Rust's borrow checker works, except without any regard for referential analysis or heap lifetimes. I personally don't think PowerShell is the right place to graft such a concept on. |
||
|
|
||
| ```powershell | ||
| # some script commands | ||
| ... | ||
| # new script block | ||
| { | ||
| # PowerShell detects and registers new IDisposable object for the script block scope | ||
| $fs = [System.IO.File]::Open("file", [System.IO.FileMode]::Open) | ||
| ... | ||
| # Here the script block is closed and | ||
| # PowerShell Engine calls implicitly $fs.Dispose() | ||
| # before destroying the scope. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How could PowerShell Engine know whether or not to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One that I know! :-)
From https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is a really good point. Even if PowerShell could prove that all of its own references to |
||
| } | ||
| ``` | ||
|
|
||
| User has no needs explicitly close a connection: | ||
|
|
||
| ```powershell | ||
| # some script commands | ||
| ... | ||
| # new script block | ||
| { | ||
| # PowerShell finds the type in ETS and registers new object for the scope | ||
| # because ETS prescribes to comply Close() to destroy an object of the type. | ||
| $db = [Database]::Open("name") | ||
| ... | ||
| # Here the script block is closed and | ||
| # PowerShell Engine calls implicitly $db.Close() | ||
| # before destroying the scope. | ||
| } | ||
|
Comment on lines
+41
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we know that a user wants this kind of functionality? This completely prevents any disposable object being provided as output or exiting any scope. As an example, I'm sure the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is typical scenario. The concept allows user to change the behavior.
No, the concept allows issue any disposable object if it is annotated.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If users need to opt-in to avoid disposal of their objects, I don't think many users will feel that's very intuitive. 😕 |
||
| ``` | ||
|
|
||
| User has no needs explicitly run a mandatory code at cmdlet destroy time: | ||
|
|
||
| ```powershell | ||
| function Get-PingReply { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Mandatory, ValueFromPipeline)] | ||
| [string] | ||
| $Target | ||
| ) | ||
| begin { | ||
| # PowerShell detects and registers new IDisposable object for the cmdlet scope | ||
| $Ping = [Ping]::new() | ||
| } | ||
| process { | ||
| $Ping.Send($Target, $Timeout, $Buffer, $PingOptions) | ||
| } | ||
| } | ||
|
|
||
| Get-PingReply '1.2.3.4' | ||
| # PowerShell run implicitly $Ping.Dispose() before destroy the cmdlet. | ||
| ``` | ||
|
|
||
| User has the ability to explicitly assign a mandatory code: | ||
|
|
||
| ```powershell | ||
| # some script commands | ||
| ... | ||
| # new script block | ||
| { | ||
| $tempFile = New-TemporaryFile | ||
|
|
||
| # Explicitly say to PowerShell Engine how release a resource before the runspace close | ||
| $tempFile.MakeDisposable([DisposableKind]::AtRunspaceClose, { script to remove the temp file } | ||
| ... | ||
| } | ||
| ``` | ||
|
|
||
| ## Specification | ||
|
|
||
| General statement is to teach `PowerShell Engine` to execute a mandatory (finally) code. | ||
|
|
||
| > Notice, .Net Runtime does not ensure finalizer/dispose code execution at application shutdown time. | ||
|
|
||
| This covers the main scenarios: | ||
|
|
||
| - call `Dispose()` for objects with `IDisposable` to avoid resource leaking | ||
| - call mandatory code for objects without `IDisposable` similar to the one that closes a database connection or deletes a temporary file. | ||
|
Comment on lines
+118
to
+119
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Database connection objects typically are
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are talking about the PowerShell module, then issuing such objects is a bad implementation.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a statement of opinion rather than fact, and I'm sure you'll find many folks who disagree quite strongly there. Just like a C# method can return an IDisposable object, why should a module in PowerShell be prevented?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can see MSFT best practice in LocalAccounts module and my port of it. There is used "fake" public objects and internally native disposable objects. This is bad for performance but exclude resources leak. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| _Our approach should be resource-centric._ | ||
| It was expressed by @BrucePay in [related issue](https://github.com/PowerShell/PowerShell/issues/6673), then in another form @lzybkr, then @alx9r. | ||
|
|
||
| We need to __register__ resources for which it is necessary to execute a mandatory (finally) code. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way this is typically done... is by implementing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concept generalizes this making PowerShell smart.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the distinction you're trying to make here. 😕 Recognising if an object is disposable is relatively easy. Recognising when disposal is appropriate is exceedingly complicated. I get the impression that too much "smart" code here will result in users feeling frustrated and that PowerShell is "dumb" to their use case. |
||
|
|
||
| This covers all scenarios described in this RFC and all other scenarios. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is too vague. When is this cleanup performed. How can it be controlled or avoided for objects or design patterns that it may hinder or completely prevent that are otherwise very useful?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concept is about magic things that is a typical behavior but allows to customize the behavior.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would we expose that customizability to users in a discoverable way that doesn't cause breaking changes to existing scripts? |
||
| It makes the `Engine` smart and brings great UX. | ||
| Old scripts and modules will benefit without any change. | ||
| No changes is needed in tools like [PowerShell EditorSyntax](https://github.com/PowerShell/EditorSyntax). | ||
|
|
||
| Try to beat any scenario as part of this resource-centric approach and you will see that the approach covers this scenario even `ForEach-Object -Parallel`) automatically (including dot sourcing). | ||
| (In fact, there can certainly be scenarios where this is impossible in principle, like remoting and serialization, but there may be tradeoffs and workarounds) | ||
|
|
||
| ### Context / Scope | ||
|
|
||
| The `Engine` should take into account Context / Scope. | ||
|
|
||
| Since variables can be copied to other scopes, the `Engine` must maintain a _usage counter_ during variable assignment / creation operations. | ||
| This adds a bit of complexity, but it should not affect performance, as in most cases, the `Engine` will only have to check a flag and only in very rare cases do extra work. | ||
| Even an `IDisposable` check needs to be done only once. | ||
| Moreover, we can even make a cache for all known types using our `TypeGen.ps1` utility. | ||
|
Comment on lines
+138
to
+141
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about assigning that variable's reference to other variables? There's a lot of hidden complexity here; it sounds like you want to essentially re-implementing .NET's existing garbage collector. Wouldn't it be better to look to supplement it rather than taking over its functionality?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See in "Context / Scope". As side note, .NET allows custom garbage collectors but I don't think we need this. Or...? :-) It is implementation details. |
||
|
|
||
| Another feature is that native objects can turn around in `PSObject` and the `Engine` should understand this too. | ||
|
|
||
| ### Annotating types / objects | ||
|
|
||
| There are four options for annotating: | ||
|
|
||
| - make a cache for all known types using our `TypeGen.ps1` utility | ||
| - use ETS to enhance some types with this feature with default options | ||
|
|
||
| For example, if we define a TempFile type then users will not have to worry about deleting temporary files created by New-TemporaryFile cmdlet at all | ||
| - embed this in the cmdlets so that they issue an already annotated object into the pipeline | ||
|
Comment on lines
+152
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would need to come with an option to configure it -- and I think the same for all the suggestions in this RFC. If we try to make this automatic, users will simply be frustrated (especially as it significantly hampers compatibility to 5.1 with scripts that _look_identical) & would thus be a massive breaking change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concept allows customizations.
If you know real example please show. This add a value to the discussion. Otherwise it is a speculation only.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Patrick gave some concrete examples in PowerShell/PowerShell#6673 on how the opt-in behaviour is problematic. You can search any number of scripts for known IDisposable types that would largely fall to pieces if this is opt-in. |
||
|
|
||
| For example, the same New-TemporaryFile or Open-Database cmdlet could do the annotation internally so that users have no need do this explicitly | ||
| - explicitly annotate like `$tempFile.MakeDisposable(DisposableKind.AtRunspaceClose, { script to remove the temp file })` | ||
|
|
||
| Note that there are many APIs that use disposable classes and now developers will have the opportunity to freely write objects of these types in the pipeline without fear of resource leakage. | ||
|
|
||
| ### Kinds of annotation | ||
|
|
||
| Mentioned `DisposableKind` is based on context / scope and could be: | ||
|
|
||
| - `Unknown`, it is default for new object. The `Engine` should annoutate the object. The default allow the `Engine` to process old code. | ||
| - `Auto`, the `Engine` annoutates based on current context/scope | ||
| - `AtGlobal`, dispose at default runspace close | ||
| - `AtModuleUnload` | ||
| - `AtRunspaceClose` | ||
| - `AtCmdletClose/AtPipelineClose` | ||
| - `AtScriptBlockClose`, dispose at current script block scope close | ||
|
|
||
| Thus, a developer is always able to set a desired behavior. | ||
|
|
||
| Moreover, we can always call MakeDisposable() in the script to adjust the behavior if the `Engine` cannot do this automatically. | ||
| In this case, we will also need a method to force the mandatory code to execute like `$tempFile.PSDispose()`. | ||
| It may also be required if we want to free an expensive resource before an end of the script block. | ||
|
Comment on lines
+160
to
+176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation being suggested isn't super clear here. Are you suggesting additional magic methods be added for this? Those tend to have discoverability problems in general, I'm not sure that's an effective way to go about it. 😕
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RFC shouldn't discuss implementation details until this is needed for understanding the concept. Prototypes exist for this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the nature of the RFC, I'm not sure how a useful discussion can come from avoiding discussion of some implementation details, to be honest. If you would like to make a prototype to demonstrate aspects, feel free.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no plans to dive into it until MSFT team says we would like to have. |
||
|
|
||
| ### Implementtation details | ||
|
|
||
| The main difficulty is that this is a generalized solution and users will expect it to work everywhere and as expected. | ||
| Thus, it is possible the implementation will be adjusted based on feedback for a long time. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This statement is concerning to me. This means that we will potentially have several versions of PowerShell with differing behaviour here, with no changes to the script they're being asked to run, correct? That sounds like a very long series of breaking changes and a general unpredictability of how a given script would operate across PowerShell versions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Experimental feature will dispel your concern.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only if we can be absolutely certain the feature is modular enough to be hidden in a feature flag. |
||
|
|
||
| From @lzybkr | ||
| > I don't think PSVariable should implement IDisposable - but as an implementation detail, maybe SessionStateScope would. | ||
|
|
||
| and | ||
| > you need two things - better disposable support and a way to register arbitrary cleanup code - this would be analogous to C# using and finally statements. | ||
|
|
||
| ## Alternate Proposals and Considerations | ||
|
|
||
| Add new `Dispose{}` script block to cmdlets. | ||
|
|
||
| This approach: | ||
|
|
||
| - only partially solves the problem of releasing up resources | ||
| - completely assigns responsibility to an user implying his high qualifications | ||
| - requires changes in Parser and tools | ||
|
Comment on lines
+189
to
+197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It assigns responsibility to the author of the code. That may be the user, or it may be the author of a given module etc. The same can be said for any C# library or code. Attempting to predict what users want instead seems like a problematic solution in my opinion. 🙂
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If we were so pessimistic it wouldn’t be worth doing anything at all. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this phrase is really the crux of the proposal: Should PowerShell attempt to do automatic resource management separately from the CLR?
I think any wide-ranging, possibly destructive feature like this can't just be something based on heuristics but must instead be based on some systematic analysis or formalism. When the garbage collector collects an object, it's based on a proof that the object is no longer in use computed by crawling the heap. When a compiler optimises an unreachable statement out, it's based on a proof that the refinement type of the condition around that statement is bottom (or something similar).
PowerShell has no such established systematic analysis, and I think adding one now would be a dramatic change to the language's semantics. It would be somewhat like adding a garbage collector to C++ today.
Instead, .NET provides a way to integrate resource disposal into their existing system for determining whether a resource is no longer in use (the GC): Implementing
Object.Finalize()to callDispose(false)..NET (and thus PowerShell) already pays a high-price for garbage collection and the analysis thereof, and I think we should position ourselves to take advantage of that.
I also worry about telling PowerShell users that they should feel free to open or acquire resources without any regard for their lifetime and that PowerShell will magically clean it up. With power comes responsibility, and we should aim to favour reliable semantics over magical ergonomics in every circumstance. Rather than try to do things for users based on what we imagine they want, we should give them easy opportunities to declare or configure what they want.