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

Enh/5943 DropIns - Prevention of File Locks and Unloading #5944

Merged
merged 9 commits into from
Sep 5, 2024

Conversation

RenatoCapelo
Copy link
Contributor

@RenatoCapelo RenatoCapelo commented Sep 5, 2024

PR Classification

New feature and code improvement.

PR Summary

Introduces an Unconfigure method for drop-ins and improves assembly loading to avoid file locking issues.

  • DropIn.cs and IDropIn.cs: Added Unconfigure method for drop-ins.
  • NuGetPackageAssemblyLoadContext.cs: Disposed packageReader to close .nupkg file.
  • AssemblyLoader.cs: Modified to load assemblies from a memory stream.
  • DropInDirectoryMonitorHostedService.cs: Added debounced unloader, async loading, and handling of file deletion events.
  • DropInDescriptor.cs: Changed to class with DynamicallyAccessedMembers attribute to suppress IL2072 warning.

Fixes #5943


This change is Reviewable

Changed DropInDescriptor from Record to Class to be able to add the suggested DynamicallyAccesedMembers decorator.
The `DropIn` class in `DropIn.cs` now includes a new `Unconfigure` method. This method handles the unconfiguration process when a drop-in is deleted. It retrieves the `ILogger<DropIn>` and `IActivityRegistry` services from the `IServiceProvider`, removes the `SampleActivity` from the activity registry, and logs the outcome.

The `IDropIn` interface in `IDropIn.cs` has been updated to include the new `Unconfigure` method, ensuring that it is called when the drop-in is deleted.

Additional using directives have been added to `DropIn.cs` for `Elsa.Workflows`, and `Microsoft.Extensions.Logging`.
Modified the `LoadPath` method in the `AssemblyLoader` class to load assemblies from a memory stream instead of directly from the file. This change prevents file locking issues by copying the file contents to a memory stream and then loading the assembly from this stream.
Ensure the packageReader is disposed of after loading the assembly from the memory stream. This change adds a call to `packageReader.Dispose()` to close the package reader and the associated `.nupkg` file, which helps in releasing resources and preventing potential file locks or memory leaks.
Updated DropInDirectoryMonitorHostedService to handle unloading of drop-ins when files are deleted. Added `_debouncedUnloader` and `_installedDropIns` fields. Modified constructor to initialize these fields. Updated `ExecuteAsync` to call `LoadDropInAssemblyAsync` initially and handle file deletion events. Added `OnDeleted` method to manage file deletions and invoke the debounced unloader. Enhanced `LoadDropInAssemblyAsync` to track loaded drop-ins. Introduced `UnloadDropInAssemblyAsync` to remove drop-ins on file deletion.
Modified the assembly loading process to use a MemoryStream
instead of a FileStream. This change ensures that the
FileStream is properly closed before the assembly is loaded,
improving resource management and preventing potential file
access issues.
Copy link

@andrenevesgomes andrenevesgomes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me. Great Job @RenatoCapelo! 👍

Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I added one small request for your consideration.

samples/drop-ins/SampleDropIn/DropIn.cs Outdated Show resolved Hide resolved
RenatoCapelo and others added 3 commits September 5, 2024 14:43
Removed logging from DropIn.Unconfigure method and added
ILogger dependency to DropInDirectoryMonitorHostedService.
Updated csproj to include Microsoft.Extensions.Logging.Abstractions.
Enhanced error handling in UnloadDropInAssemblyAsync method.
Minor formatting improvements for readability.
Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@sfmskywalker sfmskywalker merged commit c24dca1 into elsa-workflows:main Sep 5, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] DropIns - Prevention of File Locks and Unloading
3 participants