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

Dahomey.Json is currently not unloadability friendly. #63

Open
bartico6 opened this issue Oct 28, 2020 · 3 comments · May be fixed by #67
Open

Dahomey.Json is currently not unloadability friendly. #63

bartico6 opened this issue Oct 28, 2020 · 3 comments · May be fixed by #67

Comments

@bartico6
Copy link

bartico6 commented Oct 28, 2020

Short description: The use of asynclocals in
https://github.com/dahomey-technologies/Dahomey.Json/blob/master/src/Dahomey.Json/Serialization/SerializationContext.cs#L8
causes the runtime to put an instance of this variable on every thread's asynclocalmap, as seen through inspecting gc roots of each loaderallocator:
WinDbg
This in turn causes the types from Dahomey.Json to remain loaded on all thread stacks indefinitely, leading to the assembly not being unloadable.

In addition to that, the presence of static value caches ( like https://github.com/dahomey-technologies/Dahomey.Json/blob/master/src/Dahomey.Json/Util/Default.cs ) leads to any assembly cached by said classes that isn't in the same loadcontext to also become permanently loaded, as this cache is never pruned and it keeps strong references to Type objects (and instances from the assembly) which causes the assembly to become non-unloadable (this only happens if Dahomey.Json and the target assembly are in different contexts)

Setup/reproduction steps:

  1. Create a sample host app.
  2. Create a class library that uses Dahomey.Json anyhow (calling .SetupExtensions() on JsonSerializerOptions & (de)serializing something once is enough)
  3. Load the class library into its own AssemblyLoadContext.
  4. Load Dahomey.Json into any unloadable context.
  5. Call .SetupExtensions() on a JsonSerializerOptions object, then (de)serialize anything from the calling assembly.

Result:

  1. The calling assembly now has cached values in Dahomey.Json's static caches, meaning it's being held loaded by Dahomey.Json.
  2. Dahomey.Json now has the AsyncLocal static initialised, leading to it being mapped for all threads active in the app, further meaning it's being held loaded by async pinned handles & strong handles from internal thread AsyncLocalValueMaps.
    Neither of the assemblies will ever unload

If further information (or a reproduction app) is required, please let me know.

@mcatanzariti mcatanzariti linked a pull request Dec 19, 2020 that will close this issue
@mcatanzariti
Copy link
Member

@bartico6 Could please test my fixes on #67 ?

@bartico6
Copy link
Author

bartico6 commented Jan 8, 2021

Hey, sincere apologies for the delay on responding to this, I'm currently recreating the reproduction app (as I lost it during a system reinstall) and I'll get back to you soon on whether or not that fixes it 👍

@bartico6
Copy link
Author

bartico6 commented Jan 8, 2021

After checking the code for the PR you posted, and testing the new version locally, it seems it still has the issues I mentioned:
roots

I've uploaded a reproduction program to the following repository.

Obviously, replace the DLL file in References with whatever version you're testing against. It's currently a clean recompile of your #67 PR commit.

https://github.com/bartico6/Dahomey.Json.Unload

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 a pull request may close this issue.

2 participants