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

Fix #1145: Attempt Thread.currentThread().getContextClassLoader() #1146

Closed
wants to merge 3 commits into from

Conversation

melloware
Copy link

Fix #1145: Attempt Thread.currentThread().getContextClassLoader()

@theigl
Copy link
Collaborator

theigl commented Oct 29, 2024

I thought about this some more and I'm not sure if we should really go in this direction:

This would fix your issue, but at a cost. Assuming that Kryo's default class loader can never load your classes, you will get an exception for every class that you load, every time you deserialize an object graph. And you wouldn't notice because everything works as expected. If your object graph is complex and contains a large number of classes, this could result in significant overhead because throwing exceptions is expensive.

@melloware
Copy link
Author

I don't disagree with that. I didn't look what is kryo's default classloader and should it be Thread.currentThread().getContextClassLoader() ?

@theigl
Copy link
Collaborator

theigl commented Oct 29, 2024

The default classloader is defined in Kryo.java as:

    private ClassLoader classLoader = getClass().getClassLoader();

This default works fine in most environments and you are the first user to raise a class loading issue in years. So I'd say that your use-case is a bit special and there already is a solution that you can use to configure Kryo accordingly.

Given my concerns mentioned above, I tend to keep things as they are now.

@melloware melloware closed this Oct 29, 2024
@melloware melloware deleted the patch-1 branch October 29, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

DefaultClassResolver: Attempt Thread.currentThread().getContextClassloader()
2 participants