-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
/tmp/jrubyXXX.jar files are left in place by classloader if process is SIGKILLed #3352
Comments
Thinking a bit outside the box here, we could use ShrinkWrap and their ShrinkWrapClassLoader which would obviate the need for JRubyClassLoader to copy files to /tmp |
@rtyler JRubyClassLoader is mutable as it can add jars and can |
the files need to stay around as any resource loading can happen any time and the classloader does not cache the whole content in memory. using the PID to collect the jars is possible but is NO guarantee since any forking of "ruby" will reuse those jars for setting up its "forked" jruby classpath. well, it would help for those apps which do not fork. |
@mkristian Yes, I agree with the ShrinkWrap in general however, I believe will be a viable way to replace this code, most of what I'm struggling with around it is not the code so much as learning how these class fits into the rest of the LoadService. I think ShrinkWrap is the right way forward here, and it's not that difficult to use ShrinkWrap, just a tough task for me as a core newbie :) |
@rtyler the LoadService is only part of the picture. I added some javadocs to JRubyClassLoader https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/JRubyClassLoader.java#L45 the JRubyClassLoader.addURL needs to work with any possible custom url protocol. since the passed in URL might not work like this: {code}new URL(url.toString()).openStream(){code} the copy to /tmp is there. some classloaders produce URLs with "special" StreamHandler which do not work after the round trip over the String. having the URLClassLoader deal with only "file:" URLs has proven to work with all kind of custom classloaders coming with j2ee or osgi. further the URLClassLoader bits are also used to find directory entries in case there are no '.jrubydir' files on these added jars. not a big usecase IMO but was a legacy requirement to keep. the LoadService itself is not the right place and org.jruby.runtime.load.LibrarySearcher is the better or any code which uses JRubyClassLoader.addURL |
let's go the PID idea first and give more time to the ShrinkWrapClassloader idea as it is a bigger change and hard to tell where problems pop up |
When JRuby is executing via jruby-complete or another embedded type use-case, and it loads jars of some form or another a number of
/tmp/jruby*.jar
files are created.Under "normal" conditions these files stick around and are removed by something when the process is sent SIGTERM or SIGINT.
A SIGKILL however will result in the files littering the
/tmp
directory indefinitely.Reproduction Steps
java -cp ./jruby-complete-9.0.1.0.jar org.jruby.Main -e "puts 'hello'; require 'openssl'; puts 'loaded'; sleep 60"
ls /tmp | grep -i jar
Suggested solutions
It's not clear to me whether these files are needed after the classes are loaded into the runtime or not. If they are not needed, we could employ a similar fix to #1237 which was fixed with jnr/jffi#13
If they are needed for the duration of the runtime, then it would at least be useful to write the files with the PID of the Java process or something similar so a user can safely remove ones that are stale or no longer needed
The text was updated successfully, but these errors were encountered: