Skip to content

Shutdown: Unsubscribe from AppDomain event handlers #256

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

Merged
merged 4 commits into from
Jun 16, 2025

Conversation

Flohack74
Copy link
Contributor

When log4net is used in an AssemblyLoadContext that should be unloaded, log4net blocks the unloading since it subscribed to event handlers from the default load context. This PR aims to cleanup the 2 subscriptions that most likely cause this behaviour.

Related discussion here: dotnet/runtime#116142 also contains a working demo of the problem.

Note that I did not fully test the solution yet. But this cleanup is a good measure in any case.

When log4net is used in an AssemblyLoadContext that should be unloaded, log4net blocks the unloading since it subscribed to event handlers from the default load context. This PR aims to cleanup the 2 subscriptions that most likely cause this behaviour.

Note that I did not fully test the solution yet. But this cleanup is a good measure in any case.
@FreeAndNil
Copy link
Contributor

Thanks for your contribution.
I will take a deeper look on Monday.

@@ -339,6 +339,10 @@ public static ILogger GetLogger(Assembly repositoryAssembly, Type type)
/// </remarks>
public static void Shutdown()
{
//Cleanup event handlers since they only call this mathod anyways
Copy link

Choose a reason for hiding this comment

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

Suggested change
//Cleanup event handlers since they only call this mathod anyways
// Cleanup event handlers since they only call this method anyways

Copy link
Contributor

@FreeAndNil FreeAndNil left a comment

Choose a reason for hiding this comment

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

Thanks a lot.
I will fix the typo myself.

@FreeAndNil FreeAndNil merged commit 0ee8ef9 into apache:master Jun 16, 2025
3 checks passed
FreeAndNil added a commit that referenced this pull request Jun 16, 2025
@Flohack74 Flohack74 deleted the patch-1 branch June 24, 2025 15:29
@Flohack74
Copy link
Contributor Author

Thanks, great to hear. Is there a CI build or the like that I could use for further testing this out?

@FreeAndNil
Copy link
Contributor

Hi,

I'm waiting for the completion of #257.
After that, I will create 3.2.0-preview.1 and push it to nuget.org
If you don't want to wait, you can compile log4net yourself.

@Flohack74
Copy link
Contributor Author

@FreeAndNil Alright, no thats fine I will wait for preview ;)

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.

3 participants