-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Comments
Interesting, although unfortunately it is hardly possible to do something with current design. |
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. |
In order not to repeat my answer in another issue, I'll just give this link. |
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 |
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? |
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.
The text was updated successfully, but these errors were encountered: