Skip to content

Commit

Permalink
Showing 4 changed files with 18 additions and 30 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/Ruby.java
Original file line number Diff line number Diff line change
@@ -3308,7 +3308,7 @@ public void tearDown(boolean systemExit) {

getSelectorPool().cleanup();

tearDownClassLoader();
releaseClassLoader();

if (config.isProfilingEntireRun()) {
// not using logging because it's formatted
@@ -3333,9 +3333,9 @@ public void tearDown(boolean systemExit) {
}
}

private void tearDownClassLoader() {
private void releaseClassLoader() {
if (getJRubyClassLoader() != null) {
getJRubyClassLoader().tearDown(isDebug());
getJRubyClassLoader().close();
}
}

9 changes: 0 additions & 9 deletions core/src/main/java/org/jruby/embed/ScriptingContainer.java
Original file line number Diff line number Diff line change
@@ -29,16 +29,11 @@
*/
package org.jruby.embed;

import java.io.UnsupportedEncodingException;

import org.jruby.embed.internal.LocalContextProvider;

import java.io.InputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.io.Reader;
import java.io.Writer;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.HashMap;
import java.util.List;
@@ -1859,10 +1854,6 @@ public void terminate() {
LocalContextProvider provider = getProvider();
if (provider.isRuntimeInitialized()) {
provider.getRuntime().tearDown(false);
try {
getProvider().getRuntime().getJRubyClassLoader().close();
}
catch(IOException weTriedIt){}
}
provider.terminate();
}
Original file line number Diff line number Diff line change
@@ -31,9 +31,6 @@
import java.net.URLClassLoader;
import java.security.ProtectionDomain;

import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;

public class ClassDefiningJRubyClassLoader extends URLClassLoader implements ClassDefiningClassLoader {

final static ProtectionDomain DEFAULT_DOMAIN;
30 changes: 15 additions & 15 deletions core/src/main/java/org/jruby/util/JRubyClassLoader.java
Original file line number Diff line number Diff line change
@@ -104,28 +104,28 @@ public void addURL(URL url) {
}

/**
* Called when the parent runtime is torn down.
* @deprecated use {@link #close()} instead
*/
public void tearDown(boolean debug) {
close();
}

/**
* Called when the parent runtime is torn down.
*/
@Override
public void close() {
try {
super.close();
}
catch (Exception ex) { LOG.debug(ex); }

try {
// A hack to allow unloading all JDBC Drivers loaded by this classloader.
// See http://bugs.jruby.org/4226
getJDBCDriverUnloader().run();
}
catch (Exception e) {
if (debug) LOG.debug(e);
}
// if we're on Java 7+ call URLClassLoader#close :
try {
URLClassLoader.class.getMethod("close").invoke(this);
}
catch (NoSuchMethodException ex) { /* noop on Java 6 */ }
catch (IllegalAccessException ex) {
LOG.info("unexpected illegal access: ", ex);
}
catch (Exception ex) {
LOG.debug(ex);
}
catch (Exception ex) { LOG.debug(ex); }
}

@Deprecated

4 comments on commit aaa4687

@bbrowning
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this commit and the related ones before it effectively revert e3f84a0? Which means they'll break ruby processing and various other java integration uses again?

@mkristian
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kares wanted to get to this but this is essentially the same as I did before and we needed to revert this last week (before the 9k release) with e3f84a0

the reason is that it broke some use cases (swing, ruby processing, etc) - never completely understood why the runtime gets used after the teardown (maybe @bbrowning or @enebo can explain more details here). the close class loader on when terminate the ScriptingContainer yes (unless it is a Singleton runtime) but on the CLI we need keep the class loader intact.

there are at least two issues referenced in the about commit which have two use cases where the close on the JRubyClassLoader did produce failures. @bbrowning had another one.

@kares
Copy link
Member Author

@kares kares commented on aaa4687 Jul 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkristian yy - thanks I know, @bbrowning explained this to me (missed the reported issues), working on it!

@kares
Copy link
Member Author

@kares kares commented on aaa4687 Jul 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

following up, I feel like I got it right this time but please review :

this needed a revert on jruby-1_7 as well :

  • 6975482 (close is not there on Java 6)
  • backported the closing logic to happen from scripting container (like on 9K): 1c19d0c?w=1

merged this to master at e840a7d#diff-a40568b9da02963919222c8b1eb53774R3309 ... in a way that the runtime's Ruby#tearDown() no longer does JRubyClassLoader#tearDown(boolean) at all since its now deprecated and replaced (backwards-compatibly) with JRubyClassLoader#close. mostly due the fact that having 2 "release" methods is somehow confusing (and actually unnecessary) - the tearDown was kind of bad design with that boolean debug param part of the API.

also, the original JRuby class-loader tear-down attempted to release loaded JDBC drivers but the thing is (looking at issues such as #3149) that if a driver was loaded from Ruby it might still be used (as well) in those threads that keep using classes loaded by the class-loader

we're also now supporting a releaseClassLoader method on the runtime so users can easily do so

Please sign in to comment.