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

Json.Net is not currently unloadability-friendly. #2414

Open
bartico6 opened this issue Oct 22, 2020 · 5 comments
Open

Json.Net is not currently unloadability-friendly. #2414

bartico6 opened this issue Oct 22, 2020 · 5 comments

Comments

@bartico6
Copy link

bartico6 commented Oct 22, 2020

Short description:
Json.Net is currently frequently in the way of assembly unloadability. Many caches (like CachedAttributeGetter) are inaccessible to the developer (and cannot be configured) and keep types loaded indefinitely with strong references, which stops their assemblies from unloading.

CachedAttributeGetter is particularly pesky, as while other caches can be hacked at through reflection and cleared out, CachedAttributeGetter is a static generic which isn't stored in any kind of dictionary, therefore it's hidden deep in the static storage of the CLR, impossible to get at without knowing the attribute type, accessing the open generic, then closing it with said attribute.

In particular, the only way to clear CachedAttributeGetter without knowing what use it's seen is to scan all types in all load contexts of the app, find any and all Attributes then try closing the generic with them and then testing them for having any objects coming from the unloadable assembly. I think we can agree this is insanely expensive to do, because we're testing N types in the entire app for being attributes, times X members of an unloadable assembly (methods, types, fields, whatever) that can have attributes applied to them.

Loading Json.Net for each unloadable assembly is out of the question, as all other references are shared (and some have to be for type constraints between default context and unloadable assembly) and some of those references may use json.net. References are generally to be shared by design - loading duplicates would be a bad bandaid that wouldn't even work correctly for most usecases.

@sungam3r
Copy link

Interesting, although unfortunately it is hardly possible to do something with current design.

@bartico6
Copy link
Author

bartico6 commented Oct 22, 2020

A good first step would be to make CachedAttributeGetter not a static generic, but some sort of dictionary. This may slightly slow the engine down (a Dictionary of Type <-> CachedGetter instead of it being a static generic means an additional typeof(T) conversion and dictionary access instead of a JITted static access) so maybe it'd be optional.

Again, all other caches can be hacked at with Reflection but CachedAttributeGetter, as a generic static is not clearable - you'd need to qcall into the clr, if it even has some native method for discovering closed static generics (and its risking destabilizing/crashing the runtime if you don't know what you're doing, and I'm surely not an expert on CLR internals yet lol), or just do a O(N*X) where N is all types in all contexts and X is all members in unloadable assembly which frankly isn't ideal.

If this is PR material we'd probably want input from @JamesNK about whether or not a potential slowdown like this is acceptable (and what implementation of this change he'd like to see) - or if it's a thing he has an idea how to properly address.

@sungam3r
Copy link

In order not to repeat my answer in another issue, I'll just give this link.

@bartico6
Copy link
Author

I've sent you an email to continue this conversation as it's slowly leaving the issue's scope and becomes more of a "what to do next" kinda chat. I'll leave the issue open just in case someone is interested in helping me triage and resolve this

@jasonswearingen
Copy link

jasonswearingen commented Oct 8, 2024

this is a blocking issue preventing Newtonsoft.Json's use with the Godot game engine.

This is because the godot editor needs to reload project assemblies as part of the editor workflow, and this is currently blocked if using Newtonsoft.json.

The Microsoft System.Text.Json has a workaround, to manually unload the cache. is there any similar workaround that can be performed with Newtonsoft?

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

No branches or pull requests

3 participants