Automatic resource management #250

iSazonov · 2020-05-13T12:37:03Z

Initial discussion PowerShell/PowerShell#6673

vexx32

I have some concerns on this one. It's an interesting idea and may be worth exploring, but I think we would need to put some hard limitations on what we would be willing to have PowerShell attempt to do for the user here.

Too eager an approach here would likely be frustrating for a lot of users, and introduce a lot of breaking changes in otherwise perfectly fine scripts.

vexx32 · 2020-05-13T12:57:02Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


This section sounds very much like #207, I'm not clear on what this would offer that's not already captured in that RFC.

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.

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

It's also significantly less versatile

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.

Understood. I don't know how you'd go about limiting that in any appreciable manner, though.

vexx32 · 2020-05-13T13:00:47Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+```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.
+    }
+```
+
+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.
+    }


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 dbatools PowerShell module has at least one or two functions that has the purpose of creating a (disposable!) connection object that is passed to other functions so they can use it to query a database. This would completely break that module, as the connection would be closed before it can even be used.

How do we know that a user wants this kind of functionality?

It is typical scenario. The concept allows user to change the behavior.

This completely prevents any disposable object being provided as output or exiting any scope.

No, the concept allows issue any disposable object if it is annotated.

If users need to opt-in to avoid disposal of their objects, I don't think many users will feel that's very intuitive. 😕

vexx32 · 2020-05-13T13:02:25Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+
+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.
+
+As binary module developer I want to benefit from this in binary modules too.


Binary cmdlets already can implement IDisposable for this functionality.

This says that modules could issue annotated objects, and more.

How does that add value for cmdlet/compiled module authors?

vexx32 · 2020-05-13T13:03:34Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+_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.


The way this is typically done... is by implementing IDisposable. You can implement this in a PowerShell class to enable the object to be disposed in some way.

The concept generalizes this making PowerShell smart.

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.

vexx32 · 2020-05-13T13:04:19Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+
+We need to __register__ resources for which it is necessary to execute a mandatory (finally) code.
+
+This covers all scenarios described in this RFC and all other scenarios.


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?

The concept is about magic things that is a typical behavior but allows to customize the behavior.

How would we expose that customizability to users in a discoverable way that doesn't cause breaking changes to existing scripts?

vexx32 · 2020-05-13T13:05:27Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


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?

See in "Context / Scope".
Really complexity is the same as for your RFC on 90% - the same places, the same api-s.

As side note, .NET allows custom garbage collectors but I don't think we need this. Or...? :-) It is implementation details.

vexx32 · 2020-05-13T13:06:54Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+    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


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.

The concept allows customizations.

would thus be a massive breaking change

If you know real example please show. This add a value to the discussion. Otherwise it is a speculation only.

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.

vexx32 · 2020-05-13T14:00:47Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


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

RFC shouldn't discuss implementation details until this is needed for understanding the concept. Prototypes exist for this.

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.

I have no plans to dive into it until MSFT team says we would like to have.

vexx32 · 2020-05-13T14:02:23Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


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.

Experimental feature will dispel your concern.

Only if we can be absolutely certain the feature is modular enough to be hidden in a feature flag.

vexx32 · 2020-05-13T14:04:30Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


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

Attempting to predict what users want

If we were so pessimistic it wouldn’t be worth doing anything at all.

jazzdelightsme · 2020-05-14T20:18:41Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+        ...
+        # Here the script block is closed and
+        # PowerShell Engine calls implicitly $fs.Dispose()
+        # before destroying the scope.


How could PowerShell Engine know whether or not to call $fs.Dispose()? What if there was a call, [Some.DotNetType]::Method( $fs )... how does PS know if $fs is still in use or not?

One that I know! :-)

To help ensure that resources are always cleaned up appropriately, a Dispose method should be idempotent, such that it is callable multiple times without throwing an exception. Furthermore, subsequent invocations of Dispose should do nothing.

From https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose

How could PowerShell Engine know whether or not to call $fs.Dispose()?

This is a really good point. Even if PowerShell could prove that all of its own references to $fs were no longer in use (by doing something like crawling the variable stack), it wouldn't be able to see if there were remaining references to $fs beyond the veil of .NET; PowerShell doesn't own the whole runtime. The only thing that has that kind of power is the GC, and that's why you implement a finalizer for unmanaged resources. As it happens, FileStream does this already.

rjmholt · 2020-05-15T01:12:43Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

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


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 call Dispose(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.

rjmholt · 2020-05-15T01:14:28Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+
+## User Experience
+
+User has no needs explicitly dispose an object when out of the script block:


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.

rjmholt · 2020-05-15T01:24:37Z

1-Draft/RFCNNNN-Automatic-Resource-Management.md

+        ...
+        # Here the script block is closed and
+        # PowerShell Engine calls implicitly $fs.Dispose()
+        # before destroying the scope.


How could PowerShell Engine know whether or not to call $fs.Dispose()?

This is a really good point. Even if PowerShell could prove that all of its own references to $fs were no longer in use (by doing something like crawling the variable stack), it wouldn't be able to see if there were remaining references to $fs beyond the veil of .NET; PowerShell doesn't own the whole runtime. The only thing that has that kind of power is the GC, and that's why you implement a finalizer for unmanaged resources. As it happens, FileStream does this already.

JamesWTruher · 2022-03-09T23:15:05Z

While we like the intent, the implementation of CleanUp #207 (the RFC of which has been approved by the committee) and covers much of the issues raised here.

Automatic resource management

f778174

iSazonov mentioned this pull request May 13, 2020

Add new Dispose {} type block so script cmdlets have opportunity to cleanup PowerShell/PowerShell#6673

Closed

vexx32 reviewed May 13, 2020

View reviewed changes

jazzdelightsme reviewed May 14, 2020

View reviewed changes

rjmholt reviewed May 15, 2020

View reviewed changes

joeyaiello added the WG-Engine-Performance core PowerShell engine, interpreter, and runtime performance label Jul 27, 2021

JamesWTruher added the Committee-Reviewed label Mar 9, 2022

JamesWTruher closed this Mar 9, 2022

JamesWTruher added the Committee-Rejected The RFC was not accepted label Mar 9, 2022


		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.

		As binary module developer I want to benefit from this in binary modules too.


		We need to __register__ resources for which it is necessary to execute a mandatory (finally) code.

		This covers all scenarios described in this RFC and all other scenarios.

		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


		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.


		## User Experience

		User has no needs explicitly dispose an object when out of the script block:

Automatic resource management #250

Automatic resource management #250

Conversation

iSazonov commented May 13, 2020 • edited

vexx32 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vexx32 May 13, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vexx32 May 13, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iSazonov May 13, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesWTruher commented Mar 9, 2022

iSazonov commented May 13, 2020 •

edited

vexx32 May 13, 2020 •

edited

vexx32 May 13, 2020 •

edited

iSazonov May 13, 2020 •

edited