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

STORM-1853: Replace ClassLoaderObjectInputStream with ObjectInputStream #1440

Merged
merged 1 commit into from Jun 1, 2016

Conversation

abhishekagarwal87
Copy link
Contributor

No description provided.

@redsanket
Copy link

@abhishekagarwal87 There seems to be a KafkaBoltTest failure, can you take a look at it

@abhishekagarwal87
Copy link
Contributor Author

@redsanket Test failure is unrelated. Also storm-kafka tests pass on my local box.

@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented May 23, 2016

+1

I just compared ClassLoaderObjectInputStream and ObjectInputStream, and found ObjectInputStream uses latestUserDefinedLoader() which calls sun.misc.VM.latestUserDefinedLoader(). While I'm not expert on this, but at least it's not same as ClassLoader.getSystemClassLoader().

If anyone is an expert or having knowledge on classloader please explain this to avoid same issue. Thanks in advance.

@rama-nallamilli
Copy link

Not sure what the difference is but the impact of using the System Class Loader rather than userDefined means that any locally built sources that are added to the classpath by a build tool cannot be picked up (causing class not found defs). Currently attempting to build with this patch locally and will see if it resolves the issue, will update.

@rama-nallamilli
Copy link

Confirmed this patch fixed the issue for us, class loading now behaves correctly for locally built sources. Out of curiosity - this PR is for master, does that mean the fix will be part of storm 2.0.0 or is it also being reapplied to the storm 1.0.x branch? Also does anybody have any ideas when the next release cycle is for storm.

@abhishekagarwal87
Copy link
Contributor Author

This will go in 1.0.2 which is planned for a release soon.
cc @ptgoetz

@asfgit asfgit merged commit fafbaeb into apache:master Jun 1, 2016
bipinprasad pushed a commit to bipinprasad/storm that referenced this pull request Oct 17, 2019
YSTORM-6345 YSTORM-6346 prevent topo conf from overriding some system properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants