Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Isolate code that uses class loaders.
  • Loading branch information
chrisseaton committed Feb 1, 2015
1 parent 47e3adc commit 02bcdfe
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 30 deletions.
46 changes: 32 additions & 14 deletions core/src/main/java/org/jruby/Ruby.java
Expand Up @@ -52,6 +52,7 @@
import org.jruby.ext.thread.ThreadLibrary;
import org.jruby.ir.IRScriptBody;
import org.jruby.javasupport.JavaSupport;
import org.jruby.javasupport.JavaSupportImpl;
import org.jruby.lexer.yacc.ISourcePosition;
import org.jruby.parser.StaticScope;
import org.objectweb.asm.util.TraceClassVisitor;
Expand Down Expand Up @@ -97,7 +98,6 @@
import org.jruby.ir.persistence.IRReader;
import org.jruby.ir.persistence.IRReaderFile;
import org.jruby.ir.persistence.util.IRFileExpert;
import org.jruby.javasupport.JavaSupportImpl;
import org.jruby.javasupport.proxy.JavaProxyClassFactory;
import org.jruby.management.BeanManager;
import org.jruby.management.BeanManagerFactory;
Expand Down Expand Up @@ -364,6 +364,10 @@ public static boolean isGlobalRuntimeReady() {
return globalRuntime != null;
}

public static boolean isSubstrateVM() {
return false;
}

/**
* Set the global runtime to the given runtime only if it has no been set.
*
Expand Down Expand Up @@ -733,7 +737,6 @@ public IRubyObject runNormally(Node scriptNode) {
boolean compile = getInstanceConfig().getCompileMode().shouldPrecompileCLI();
if (compile || config.isShowBytecode()) {
scriptAndCode = precompileCLI(scriptNode);

}

if (scriptAndCode != null) {
Expand Down Expand Up @@ -1211,7 +1214,7 @@ private void init() {
// Construct key services
loadService = config.createLoadService(this);
posix = POSIXFactory.getPOSIX(new JRubyPOSIXHandler(this), config.isNativeEnabled());
javaSupport = new JavaSupportImpl(this);
javaSupport = loadJavaSupport();

executor = new ThreadPoolExecutor(
RubyInstanceConfig.POOL_MIN,
Expand Down Expand Up @@ -1272,13 +1275,7 @@ private void init() {

// load JRuby internals, which loads Java support
// if we can't use reflection, 'jruby' and 'java' won't work; no load.
boolean reflectionWorks;
try {
ClassLoader.class.getDeclaredMethod("getResourceAsStream", String.class);
reflectionWorks = true;
} catch (Exception e) {
reflectionWorks = false;
}
boolean reflectionWorks = doesReflectionWork();

if (!RubyInstanceConfig.DEBUG_PARSER && reflectionWorks
&& getInstanceConfig().getCompileMode() != CompileMode.TRUFFLE) {
Expand Down Expand Up @@ -1309,7 +1306,7 @@ && getInstanceConfig().getCompileMode() != CompileMode.TRUFFLE) {
}

if (config.getLoadGemfile()) {
loadService.loadFromClassLoader(getClassLoader(), "jruby/bundler/startup.rb", false);
loadBundler();
}

setNetworkStack();
Expand All @@ -1323,6 +1320,23 @@ && getInstanceConfig().getCompileMode() != CompileMode.TRUFFLE) {
}
}

public JavaSupport loadJavaSupport() {
return new JavaSupportImpl(this);
}

private void loadBundler() {
loadService.loadFromClassLoader(getClassLoader(), "jruby/bundler/startup.rb", false);
}

private boolean doesReflectionWork() {
try {
ClassLoader.class.getDeclaredMethod("getResourceAsStream", String.class);
return true;
} catch (Exception e) {
return false;
}
}

private void bootstrap() {
initCore();
initExceptions();
Expand Down Expand Up @@ -3284,9 +3298,7 @@ public void tearDown(boolean systemExit) {

getSelectorPool().cleanup();

if (getJRubyClassLoader() != null) {
getJRubyClassLoader().tearDown(isDebug());
}
tearDownClassLoader();

if (config.isProfilingEntireRun()) {
// not using logging because it's formatted
Expand All @@ -3311,6 +3323,12 @@ public void tearDown(boolean systemExit) {
}
}

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

/**
* TDOD remove the synchronized. Synchronization should be a implementation detail of the ProfilingService.
* @param profileData
Expand Down
18 changes: 16 additions & 2 deletions core/src/main/java/org/jruby/RubyInstanceConfig.java
Expand Up @@ -390,7 +390,7 @@ public InputStream getScriptSource() {
if (script.startsWith("file:") && script.indexOf(".jar!/") != -1) {
stream = new URL("jar:" + script).openStream();
} else if (script.startsWith("classpath:")) {
stream = Ruby.getClassLoader().getResourceAsStream(script.substring("classpath:".length()));
stream = getScriptSourceFromJar(script);
} else {
File file = JRubyFile.create(getCurrentDirectory(), getScriptFileName());
if (isXFlag()) {
Expand All @@ -414,6 +414,10 @@ public InputStream getScriptSource() {
}
}

private InputStream getScriptSourceFromJar(String script) {
return Ruby.getClassLoader().getResourceAsStream(script.substring("classpath:".length()));
}

private static InputStream findScript(File file) throws IOException {
StringBuffer buf = new StringBuffer();
BufferedReader br = new BufferedReader(new FileReader(file));
Expand Down Expand Up @@ -1401,6 +1405,16 @@ public String getProfilingService() {
public void setProfilingService( String service ) {
this.profilingService = service;
}

private static ClassLoader setupLoader() {
final ClassLoader thisLoader = RubyInstanceConfig.class.getClassLoader();

if (thisLoader == null) {
return null;
} else {
return Thread.currentThread().getContextClassLoader();

This comment has been minimized.

Copy link
@mkristian

mkristian Feb 1, 2015

Member

this is wrong in every case where RubyInstanceConfig.class.getClassLoader() is NOT the Thread.currentThread.contextClassLoader. you end up with classloader which can not load the jruby kernel (require 'java' and require 'jruby')

and that are the failing tests on travis (at least partially)

jruby just works fine without any Thread.currentThread.contextClassLoader involved - like OSGi frameworks.

}
}

////////////////////////////////////////////////////////////////////////////
// Configuration fields.
Expand Down Expand Up @@ -1446,7 +1460,7 @@ public void setProfilingService( String service ) {
private ProfileOutput profileOutput = new ProfileOutput(System.err);
private String profilingService;

private ClassLoader thisLoader = RubyInstanceConfig.class.getClassLoader();
private ClassLoader thisLoader = setupLoader();
// thisLoader can be null for example when jruby comes from the boot-classLoader
private ClassLoader loader = thisLoader == null ? Thread.currentThread().getContextClassLoader() : thisLoader;

This comment has been minimized.

Copy link
@mkristian

mkristian Feb 1, 2015

Member

with the above setupLoader() we always end up the Thread.currentThread().getContextClassLoader()

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Feb 1, 2015

Author Contributor

Thanks - this is fixed in 46e1058 (I saw the Travis failure).


Expand Down
13 changes: 11 additions & 2 deletions core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java
Expand Up @@ -641,8 +641,9 @@ private String resolveScript(String scriptName) {
} catch (Exception e) {
// keep going, try PATH
}
if(Ruby.getClassLoader().getResourceAsStream("bin/" + scriptName) != null){
return "classpath:bin/" + scriptName;
String resolved = resolveScriptUsingClassLoader(scriptName);
if (resolved != null) {
return resolved;
}
try {
Object pathObj = config.getEnvironment().get("PATH");
Expand All @@ -667,6 +668,14 @@ private String resolveScript(String scriptName) {
return null;
}

public String resolveScriptUsingClassLoader(String scriptName) {
if(Ruby.getClassLoader().getResourceAsStream("bin/" + scriptName) != null){
return "classpath:bin/" + scriptName;
} else {
return null;
}
}

private String grabValue(String errorMessage) {
String optValue = grabOptionalValue();
if (optValue != null) {
Expand Down
Expand Up @@ -49,7 +49,7 @@ public SubstrateNode(SubstrateNode prev) {

@Specialization
public boolean substrate() {
return getContext().isSubstrate();
return getContext().getRuntime().isSubstrateVM();
}

}
Expand Down
Expand Up @@ -423,8 +423,4 @@ public RubiniusPrimitiveManager getRubiniusPrimitiveManager() {
return rubiniusPrimitiveManager;
}

public boolean isSubstrate() {
return false;
}

}
Expand Up @@ -37,6 +37,7 @@

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand Down Expand Up @@ -424,20 +425,28 @@ public void loadRubyCore(String fileName) {
final Source source;

try {
final LoadServiceResource resource = context.getRuntime().getLoadService().getClassPathResource(context.getRuntime().getJRubyClassLoader(), fileName);

if (resource == null) {
throw new RuntimeException("couldn't load Truffle core library " + fileName);
}

source = Source.fromReader(new InputStreamReader(resource.getInputStream(), StandardCharsets.UTF_8), "core:/" + fileName);
source = Source.fromReader(new InputStreamReader(getRubyCoreInputStream(fileName), StandardCharsets.UTF_8), "core:/" + fileName);
} catch (IOException e) {
throw new RuntimeException(e);
}

context.load(source, null, NodeWrapper.IDENTITY);
}

public InputStream getRubyCoreInputStream(String fileName) {
final LoadServiceResource resource = context.getRuntime().getLoadService().getClassPathResource(context.getRuntime().getJRubyClassLoader(), fileName);

if (resource == null) {
throw new RuntimeException("couldn't load Truffle core library " + fileName);
}

try {
return resource.getInputStream();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

public void initializeEncodingConstants() {
getContext().getRuntime().getEncodingService().defineEncodings(new EncodingService.EncodingDefinitionVisitor() {
@Override
Expand Down

0 comments on commit 02bcdfe

Please sign in to comment.