Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 0f305773e07c^
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 764d6af8ad81
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Apr 1, 2015

  1. Rework load sequence wrt feature checking and locking. 

    Band-aid for #2788.
    @headius
    headius committed Apr 1, 2015
    Copy the full SHA
    0f30577 View commit details

  2. Rework jar service loading to properly build path. 

    The logic here will walk from end to beginning of the loading jar
filename, building service class names and attempting to load them.
This is an O(n) operation for the number of path elements, but I
do not know another way to deal with RubyGems canonicalizing jar
file path names.
    @headius
    headius committed Apr 1, 2015
    Copy the full SHA
    764d6af View commit details
Showing with 102 additions and 98 deletions.
  1. +43 −28 core/src/main/java/org/jruby/runtime/load/ClassExtensionLibrary.java
  2. +59 −70 core/src/main/java/org/jruby/runtime/load/LoadService.java
71 changes: 43 additions & 28 deletions core/src/main/java/org/jruby/runtime/load/ClassExtensionLibrary.java
View file
Original file line number Diff line number Diff line change
@@ -27,7 +27,6 @@
***** END LICENSE BLOCK *****/
package org.jruby.runtime.load;

import jnr.posix.JavaSecuredFile;
import org.jruby.Ruby;

import java.net.URL;
@@ -66,42 +65,58 @@ static ClassExtensionLibrary tryFind(Ruby runtime, String searchName) {
// Create package name, by splitting on / and joining all but the last elements with a ".", and downcasing them.
String[] all = searchName.split("/");

StringBuilder finName = new StringBuilder();
for (int i = 0, j = (all.length - 1); i < j; i++) {
finName.append(all[i].toLowerCase()).append(".");
}
// make service name out of last element
String serviceName = buildServiceName(all[all.length - 1]);

try {
// Make the class name look nice, by splitting on _ and capitalize each segment, then joining
// the, together without anything separating them, and last put on "Service" at the end.
String[] last = all[all.length - 1].split("_");
for (int i = 0, j = last.length; i < j; i++) {
if ("".equals(last[i])) break;
finName.append(Character.toUpperCase(last[i].charAt(0))).append(last[i].substring(1));
}
finName.append("Service");
// allocate once with plenty of space, to reduce object churn
StringBuilder classNameBuilder = new StringBuilder(searchName.length() * 2);
StringBuilder classFileBuilder = new StringBuilder(searchName.length() * 2);

for (int i = all.length - 2; i >= 0; i--) {
buildClassName(classNameBuilder, classFileBuilder, all, i, serviceName);

// We don't want a package name beginning with dots, so we remove them
String className = finName.toString().replaceAll("^\\.*", "");
String classFile = className.replaceAll("\\.", "/") + ".class";
// look for the filename in classloader resources
URL resource = runtime.getJRubyClassLoader().getResource(classFileBuilder.toString());
if (resource == null) continue;

// quietly try to load the class, which must be reachable as a .class resource
URL resource = runtime.getJRubyClassLoader().getResource(classFile);
if (resource != null) {
String className = classNameBuilder.toString();

try {
Class theClass = runtime.getJavaSupport().loadJavaClass(className);
return new ClassExtensionLibrary(className + ".java", theClass);
} catch (ClassNotFoundException cnfe) {
// file was found but class couldn't load; continue to next package segment
continue;
} catch (UnsupportedClassVersionError ucve) {
if (runtime.isDebug()) ucve.printStackTrace();
throw runtime.newLoadError("JRuby ext built for wrong Java version in `" + className + "': " + ucve, className.toString());
}
}

// not found
return null;
}

return null;
} catch (ClassNotFoundException cnfe) {
if (runtime.isDebug()) cnfe.printStackTrace();
private static void buildClassName(StringBuilder nameBuilder, StringBuilder fileBuilder, String[] all, int i, String serviceName) {
nameBuilder.setLength(0);
fileBuilder.setLength(0);
for (int j = i; j < all.length - 1; j++) {
nameBuilder.append(all[j]).append(".");
fileBuilder.append(all[j]).append("/");
}
nameBuilder.append(serviceName);
fileBuilder.append(serviceName).append(".class");
}

// So apparently the class doesn't exist
return null;
} catch (UnsupportedClassVersionError ucve) {
if (runtime.isDebug()) ucve.printStackTrace();
throw runtime.newLoadError("JRuby ext built for wrong Java version in `" + finName + "': " + ucve, finName.toString());
private static String buildServiceName(String jarName) {
String[] last = jarName.split("_");
StringBuilder serviceName = new StringBuilder();
for (int i = 0, j = last.length; i < j; i++) {
if ("".equals(last[i])) break;
serviceName.append(Character.toUpperCase(last[i].charAt(0))).append(last[i].substring(1));
}
serviceName.append("Service");
return serviceName.toString();
}

public ClassExtensionLibrary(String name, Class extension) {
129 changes: 59 additions & 70 deletions core/src/main/java/org/jruby/runtime/load/LoadService.java
View file
Original file line number Diff line number Diff line change
@@ -396,44 +396,22 @@ private RequireState requireCommon(String requireName, boolean circularRequireWa
return RequireState.ALREADY_LOADED;
}

if (!requireLocks.lock(requireName)) {
if (circularRequireWarning && runtime.isVerbose()) {
warnCircularRequire(requireName);
}
return RequireState.CIRCULAR;
if (!runtime.getProfile().allowRequire(requireName)) {
throw runtime.newLoadError("no such file to load -- " + requireName, requireName);
}
try {
if (!runtime.getProfile().allowRequire(requireName)) {
throw runtime.newLoadError("no such file to load -- " + requireName, requireName);
}

// check for requiredName again now that we're locked
if (featureAlreadyLoaded(requireName)) {
return RequireState.ALREADY_LOADED;
}

// numbers from loadTimer does not include lock waiting time.
long startTime = loadTimer.startLoad(requireName);
try {
boolean loaded = smartLoadInternal(requireName);
return loaded ? RequireState.LOADED : RequireState.ALREADY_LOADED;
} finally {
loadTimer.endLoad(requireName, startTime);
}
} finally {
requireLocks.unlock(requireName);
}
return smartLoadInternal(requireName, circularRequireWarning);
}

protected final RequireLocks requireLocks = new RequireLocks();

private class RequireLocks {
private final Map<String, ReentrantLock> pool;
private final ConcurrentHashMap<String, ReentrantLock> pool;
// global lock for require must be fair
private final ReentrantLock globalLock;

private RequireLocks() {
this.pool = new HashMap<String, ReentrantLock>();
this.pool = new ConcurrentHashMap<>(8, 0.75f, 2);
this.globalLock = new ReentrantLock(true);
}

@@ -448,35 +426,19 @@ private RequireLocks() {
* returns false without getting a lock. Otherwise true.
*/
private boolean lock(String requireName) {
ReentrantLock lock;

while (true) {
synchronized (pool) {
lock = pool.get(requireName);
if (lock == null) {
if (runtime.getInstanceConfig().isGlobalRequireLock()) {
lock = globalLock;
} else {
lock = new ReentrantLock();
}
pool.put(requireName, lock);
} else if (lock.isHeldByCurrentThread()) {
return false;
}
}
ReentrantLock lock = pool.get(requireName);

lock.lock();

// repeat until locked object still in requireLocks.
synchronized (pool) {
if (pool.get(requireName) == lock) {
// the object is locked && the lock is in the pool
return true;
}
// go next try
lock.unlock();
}
if (lock == null) {
ReentrantLock newLock = new ReentrantLock();
lock = pool.putIfAbsent(requireName, newLock);
if (lock == null) lock = newLock;
}

if (lock.isHeldByCurrentThread()) return false;

lock.lock();

return true;
}

/**
@@ -486,13 +448,11 @@ private boolean lock(String requireName) {
* name of the lock to be unlocked.
*/
private void unlock(String requireName) {
synchronized (pool) {
ReentrantLock lock = pool.get(requireName);
if (lock != null) {
assert lock.isHeldByCurrentThread();
lock.unlock();
pool.remove(requireName);
}
ReentrantLock lock = pool.get(requireName);

if (lock != null) {
assert lock.isHeldByCurrentThread();
lock.unlock();
}
}
}
@@ -516,26 +476,55 @@ public boolean smartLoad(String file) {
return require(file);
}

private boolean smartLoadInternal(String file) {
private RequireState smartLoadInternal(String file, boolean circularRequireWarning) {
checkEmptyLoad(file);
SearchState state = findFileForLoad(file);
if (state == null) {
return false;

// check with short name
if (featureAlreadyLoaded(file)) {
return RequireState.ALREADY_LOADED;
}

SearchState state = findFileForLoad(file);

if (state.library == null) {
throw runtime.newLoadError("no such file to load -- " + state.searchFile, state.searchFile);
}

// check with long name
if (featureAlreadyLoaded(state.loadName)) {
return false;
return RequireState.ALREADY_LOADED;
}

if (!requireLocks.lock(state.loadName)) {
if (circularRequireWarning && runtime.isVerbose()) {
warnCircularRequire(state.loadName);
}
return RequireState.CIRCULAR;
}

boolean loaded = tryLoadingLibraryOrScript(runtime, state);
if (loaded) {
addLoadedFeature(file, state.loadName);
// numbers from loadTimer does not include lock waiting time.
long startTime = loadTimer.startLoad(state.loadName);

try {
// check with short name again
if (featureAlreadyLoaded(file)) {
return RequireState.ALREADY_LOADED;
}

// check with long name again in case it loaded while we were locking
if (featureAlreadyLoaded(state.loadName)) {
return RequireState.ALREADY_LOADED;
}

boolean loaded = tryLoadingLibraryOrScript(runtime, state);
if (loaded) {
addLoadedFeature(file, state.loadName);
}
return loaded ? RequireState.LOADED : RequireState.ALREADY_LOADED;
} finally {
loadTimer.endLoad(state.loadName, startTime);
requireLocks.unlock(state.loadName);
}
return loaded;
}

private static class LoadTimer {