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.

Sign up for GitHub

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

Jump to bottom

Add clean block to script block as a peer to begin, process, and end to allow easy resource cleanup #15177

Merged
merged 36 commits into from Oct 11, 2021
Merged

Add clean block to script block as a peer to begin, process, and end to allow easy resource cleanup #15177

merged 36 commits into from Oct 11, 2021

Commits on Oct 11, 2021

  1. ✨ Add cleanup block

    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    aabc7f8 View commit details
    Browse the repository at this point in the history

  2. ♻️ Refactor & address code review 

     - Move cleanup invocation into discrete method for CompiledScriptBlock
 - Tidy code paths in CompiledScriptBlock and DlrScriptCommandProcessor
 - Finish renaming work from dispose->cleanup
 - Rename critical region->critical section
 - Refactor disposal in CommandProcessorBase to avoid unnecessary paths
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    ed43b50 View commit details
    Browse the repository at this point in the history

  3. ♻️ Refactor pipeline handling a bit

    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    b9cbf8b View commit details
    Browse the repository at this point in the history

  4. 🎨 Linter fixes

    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    dbc2b6c View commit details
    Browse the repository at this point in the history

  5. ♻️ Refactor dispose in CommandProcessorBase 

     - Rename helper method.
 - Ensure DlrScriptCommandProcessor can't invoke Cleanup more than once
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    480eb01 View commit details
    Browse the repository at this point in the history

  6. 🔧 Adjust scope and error handling 

    - 🐛 fix error duplication
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    e86422b View commit details
    Browse the repository at this point in the history

  7. ♻️ Avoid double type check for cleanup 

    Moving half of the cleanup logic to DlrScriptCommandProcessor lets us
avoid the double type check/cast and handle script functions exclusively
in DlrScriptCommandProcessor.

Remaining branch for PSScriptCmdlet is retained in the base class's
`Dispose()` method itself (moved up the stack one method).
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    506b3b2 View commit details
    Browse the repository at this point in the history

  8. ♻️ Address Rob's feedback 

    - Explicit type for variable declaration
- Ensure we mark cleanup as completed if there is no cleanup block.
- Check depth before taking the lock
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    cb9491a View commit details
    Browse the repository at this point in the history

  9. ♻️ Move duplicated scope handling to method 

    This method could also be utilised in all the Begin/Process/End paths.
However, they each have a very slightly different version of the code,
do things in different orders, and split up certain parts of the code.

We can revisit whether we want to refactor those code paths to use the
new method in future, but for now this avoids duplicating the code from
here to DlrScriptCommandProcessor, which would introduce complications.
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    0d76834 View commit details
    Browse the repository at this point in the history

  10. ♻️ Refactor critical section logic 

    With some review changes, it appears we're able to use Monitor methods
instead of a semaphore as was previously required.

Also added some comments.
    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    15b5485 View commit details
    Browse the repository at this point in the history

  11. ♻️ Address Dongbo's review comments

    @vexx32 @daxian-dbw
    vexx32 authored and daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    47fd6d2 View commit details
    Browse the repository at this point in the history

  12. Refactor the design of 'Clean' block implementation

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    5e9f75f View commit details
    Browse the repository at this point in the history

  13. Revert a few unneeded changes

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    9704c7d View commit details
    Browse the repository at this point in the history

  14. Fix build and avoid using 'CriticalSection' code for now

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    376033a View commit details
    Browse the repository at this point in the history

  15. Fix exception catching regression

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    d9eac3b View commit details
    Browse the repository at this point in the history

  16. Fix test failures

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    dc01c2b View commit details
    Browse the repository at this point in the history

  17. Fix pipeline regressions

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    aa57e6b View commit details
    Browse the repository at this point in the history

  18. Update the comments to make it more clear

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    771dba7 View commit details
    Browse the repository at this point in the history

  19. Complete a partial comment

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    bab8f85 View commit details
    Browse the repository at this point in the history

  20. Changes to SteppablePipeline to support 'Clean' block

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    98339f5 View commit details
    Browse the repository at this point in the history

  21. Clean up the steppable pipeline changes and add more comments

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    8d5e053 View commit details
    Browse the repository at this point in the history

  22. Add lots of tests

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    5c8cc9a View commit details
    Browse the repository at this point in the history

  23. Add additional functional tests for steppable pipeline

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    c9634e6 View commit details
    Browse the repository at this point in the history

  24. Rename the new block from 'cleanup' to 'clean'

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    7f18442 View commit details
    Browse the repository at this point in the history

  25. Fix 3 tests and a comment

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    35e466b View commit details
    Browse the repository at this point in the history

  26. Update a couple test comments

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    deadba7 View commit details
    Browse the repository at this point in the history

  27. Add 2 more tests for 'exit' scenarios

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    e30736a View commit details
    Browse the repository at this point in the history

  28. Address Ilya's comments

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    a2c9174 View commit details
    Browse the repository at this point in the history

  29. Add some more changes: 

    1. not support directly invoking the 'clean' block of a script block.
2. not support a script block with the 'clean' block for '.ForEach' magic method.
3. make 'clean' block mimic the 'finally' clause
4. add a few more tests
    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    7dbe15b View commit details
    Browse the repository at this point in the history

  30. Address new comments from Ilya

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    aef7d95 View commit details
    Browse the repository at this point in the history

  31. Update comments to make the code easier to understand

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    451830d View commit details
    Browse the repository at this point in the history

  32. Update tests to use better names

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    7ca9b4c View commit details
    Browse the repository at this point in the history

  33. Address comments from Paul and Joel

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    0060e0d View commit details
    Browse the repository at this point in the history

  34. Address Aditya's comment

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    21538bb View commit details
    Browse the repository at this point in the history

  35. Revert changes in ImplicitRemotingCommands.cs because they are not ne… 

    …eded
    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    535584c View commit details
    Browse the repository at this point in the history

  36. Guard the 'clean' block behind an experimental feature

    @daxian-dbw
    daxian-dbw committed Oct 11, 2021
    Copy the full SHA
    93305d4 View commit details
    Browse the repository at this point in the history