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: fe45447df641
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 330e5ee25917
Choose a head ref
  • 2 commits
  • 2 files changed
  • 2 contributors

Commits on Nov 14, 2016

  1. Catch LinkageError during DynamicScope generation. 

    When multiple threads are generating the same DynamicScope class, both
threads may try to generate the same class at the same time. When the second
thread hits defineClass, it throws a LinkageError becuase the class has already
been defined.

With this change we catch this exception and call loadClass to get the class that
the previous thread created.
    @snowp @headius
    snowp authored and headius committed Nov 14, 2016
    Copy the full SHA
    44e69de View commit details

  2. Synchronize DynamicScope generation to avoid LinkageError. 

    This is an enhancement to #4285 to avoid the exception altogether.

Any remaining exceptions that bubble out are intended to do so, so
we can see them and get reports and fix them. This modification
should effectively prevent double-loading (which caused the
LinkageError) but have reduced overhead.
    @headius
    headius committed Nov 14, 2016
    Copy the full SHA
    330e5ee View commit details
Showing with 72 additions and 13 deletions.
  1. +42 −11 core/src/main/java/org/jruby/runtime/scope/DynamicScopeGenerator.java
  2. +30 −2 core/src/test/java/org/jruby/runtime/TestDynamicScope.java
53 changes: 42 additions & 11 deletions core/src/main/java/org/jruby/runtime/scope/DynamicScopeGenerator.java
View file
Original file line number Diff line number Diff line change
@@ -78,7 +78,39 @@ public static MethodHandle generate(final int size) {
final String clsPath = "org/jruby/runtime/scopes/DynamicScope" + size;
final String clsName = clsPath.replaceAll("/", ".");

// try to load the class, in case we have parallel generation happening
Class p;

try {
p = CDCL.loadClass(clsName);
} catch (ClassNotFoundException cnfe) {
// try again under lock
synchronized (CDCL) {
try {
p = CDCL.loadClass(clsName);
} catch (ClassNotFoundException cnfe2) {
// proceed to actually generate the class
p = generateInternal(size, clsPath, clsName);
}
}
}

// acquire constructor handle and store it
try {
MethodHandle mh = MethodHandles.lookup().findConstructor(p, MethodType.methodType(void.class, StaticScope.class, DynamicScope.class));
mh = mh.asType(MethodType.methodType(DynamicScope.class, StaticScope.class, DynamicScope.class));
MethodHandle previousMH = specializedFactories.putIfAbsent(size, mh);
if (previousMH != null) mh = previousMH;

return mh;
} catch (Exception e) {
throw new RuntimeException(e);
}
}

private static Class generateInternal(final int size, final String clsPath, final String clsName) {
// ensure only one thread will attempt to generate and define the new class
synchronized (CDCL) {
// create a new one
final String[] newFields = varList(size);

@@ -255,16 +287,7 @@ public static MethodHandle generate(final int size) {
}});
}};

Class p = defineClass(jiteClass);

MethodHandle mh = MethodHandles.lookup().findConstructor(p, MethodType.methodType(void.class, StaticScope.class, DynamicScope.class));
mh = mh.asType(MethodType.methodType(DynamicScope.class, StaticScope.class, DynamicScope.class));
MethodHandle previousMH = specializedFactories.putIfAbsent(size, mh);
if (previousMH != null) mh = previousMH;

return mh;
} catch (Exception e) {
throw new RuntimeException(e);
return defineClass(jiteClass);
}
}

@@ -310,7 +333,15 @@ private static MethodHandle getClassFromSize(int size) {
}

private static Class defineClass(JiteClass jiteClass) {
return CDCL.defineClass(jiteClass.getClassName().replaceAll("/", "."), jiteClass.toBytes(JDKVersion.V1_7));
return CDCL.defineClass(classNameFromJiteClass(jiteClass), jiteClass.toBytes(JDKVersion.V1_7));
}

private static Class loadClass(JiteClass jiteClass) throws ClassNotFoundException {
return CDCL.loadClass(classNameFromJiteClass(jiteClass));
}

private static String classNameFromJiteClass(JiteClass jiteClass) {
return jiteClass.getClassName().replaceAll("/", ".");
}

private static String[] varList(int size) {
32 changes: 30 additions & 2 deletions core/src/test/java/org/jruby/runtime/TestDynamicScope.java
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,43 @@
package org.jruby.runtime;

import java.lang.invoke.MethodHandle;
import java.util.ArrayList;
import junit.framework.TestCase;
import org.jruby.RubyBasicObject;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.scope.DynamicScopeGenerator;
import org.junit.Assert;
import org.junit.Test;

import java.util.Arrays;

public class TestDynamicScope extends TestCase {
@Test
public void testMultithreadedScopeGeneration() throws Throwable {
final ArrayList<MethodHandle> result = new ArrayList<>();
final int numberOfThreads = 10;

Thread[] threads = new Thread[numberOfThreads];
for(int i = 0; i < numberOfThreads; i++) {
threads[i] = new Thread(new Runnable(){
public void run() {
MethodHandle dynamicScope = DynamicScopeGenerator.generate(10);
synchronized(result) {
result.add(dynamicScope);
}
}
});
threads[i].start();
}

for(int i = 0; i < numberOfThreads; i++) {
threads[i].join();
}

Assert.assertEquals(numberOfThreads, result.size());
for (int i = 0; i < result.size() - 1; ++i) {
Assert.assertEquals(result.get(i), result.get(i+1));
}
}

@Test
public void testScopeGeneration() throws Throwable {
for (int i = 0; i < 100; i++) {