Isolation group classloader cleanup - this fixes #3017

This commit is contained in:
Julien Viet
2019-07-02 16:34:50 +02:00
parent 9f0b710d01
commit 4e8f40b078
4 changed files with 77 additions and 11 deletions

View File

@@ -26,6 +26,7 @@ import io.vertx.core.spi.VerticleFactory;
import io.vertx.core.spi.metrics.VertxMetrics;
import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
@@ -33,6 +34,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
@@ -40,7 +42,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -57,7 +58,7 @@ public class DeploymentManager {
private final VertxInternal vertx;
private final Map<String, Deployment> deployments = new ConcurrentHashMap<>();
private final Map<String, ClassLoader> classloaders = new WeakHashMap<>();
private final Map<String, IsolatingClassLoader> classloaders = new HashMap<>();
private final Map<String, List<VerticleFactory>> verticleFactories = new ConcurrentHashMap<>();
private final List<VerticleFactory> defaultFactories = new ArrayList<>();
@@ -389,8 +390,8 @@ public class DeploymentManager {
// IMPORTANT - Isolation groups are not supported on Java 9+, because the system classloader is not an URLClassLoader
// anymore. Thus we can't extract the paths from the classpath and isolate the loading.
synchronized (this) {
cl = classloaders.get(isolationGroup);
if (cl == null) {
IsolatingClassLoader icl = classloaders.get(isolationGroup);
if (icl == null) {
ClassLoader current = getCurrentClassLoader();
if (!(current instanceof URLClassLoader)) {
throw new IllegalStateException("Current classloader must be URLClassLoader");
@@ -414,10 +415,13 @@ public class DeploymentManager {
urls.addAll(Arrays.asList(urlc.getURLs()));
// Create an isolating cl with the urls
cl = new IsolatingClassLoader(urls.toArray(new URL[urls.size()]), getCurrentClassLoader(),
options.getIsolatedClasses());
classloaders.put(isolationGroup, cl);
icl = new IsolatingClassLoader(urls.toArray(new URL[urls.size()]), getCurrentClassLoader(),
options.getIsolatedClasses());
classloaders.put(isolationGroup, icl);
} else {
icl.refCount++;
}
cl = icl;
}
}
return cl;
@@ -639,6 +643,20 @@ public class DeploymentManager {
// Log error but we report success anyway
log.error("Failed to run close hook", ar2.cause());
}
String group = options.getIsolationGroup();
if (group != null) {
synchronized (DeploymentManager.this) {
IsolatingClassLoader icl = classloaders.get(group);
if (--icl.refCount == 0) {
classloaders.remove(group);
try {
icl.close();
} catch (IOException e) {
log.debug("Issue when closing isolation group loader", e);
}
}
}
}
if (ar.succeeded() && undeployCount.incrementAndGet() == numToUndeploy) {
reportSuccess(null, undeployingContext, completionHandler);
} else if (ar.failed() && !failureReported.get()) {

View File

@@ -24,11 +24,14 @@ import java.util.List;
*/
public class IsolatingClassLoader extends URLClassLoader {
private volatile boolean closed;
private List<String> isolatedClasses;
int refCount;
public IsolatingClassLoader(URL[] urls, ClassLoader parent, List<String> isolatedClasses) {
super(urls, parent);
this.isolatedClasses = isolatedClasses;
this.refCount = 1;
}
@Override
@@ -122,6 +125,16 @@ public class IsolatingClassLoader extends URLClassLoader {
return Collections.enumeration(resources);
}
@Override
public void close() throws IOException {
closed = true;
super.close();
}
public boolean isClosed() {
return closed;
}
private boolean isVertxOrSystemClass(String name) {
return
name.startsWith("java.") ||

View File

@@ -1095,14 +1095,24 @@ public class DeploymentTest extends VertxTestBase {
String dir = createClassOutsideClasspath("MyVerticle");
List<String> extraClasspath = Arrays.asList(dir);
try {
vertx.deployVerticle("java:" + ExtraCPVerticleNotInParentLoader.class.getCanonicalName(), new DeploymentOptions().setIsolationGroup("somegroup").
setExtraClasspath(extraClasspath), onSuccess(v -> {
testComplete();
DeploymentOptions options = new DeploymentOptions().setIsolationGroup("somegroup").setExtraClasspath(extraClasspath);
vertx.deployVerticle("java:" + ExtraCPVerticleNotInParentLoader.class.getCanonicalName(), options, onSuccess(id1 -> {
vertx.deployVerticle("java:" + ExtraCPVerticleNotInParentLoader.class.getCanonicalName(), options, onSuccess(id2 -> {
vertx.undeploy(id1, onSuccess(v1 -> {
assertFalse(ExtraCPVerticleNotInParentLoader.cl.isClosed());
vertx.undeploy(id2, onSuccess(v2 -> {
assertTrue(ExtraCPVerticleNotInParentLoader.cl.isClosed());
testComplete();
}));
}));
}));
}));
assertTrue(expectedSuccess);
await();
} catch (IllegalStateException e) {
assertFalse(expectedSuccess);
} finally {
ExtraCPVerticleNotInParentLoader.cl = null;
}
}
@@ -1125,6 +1135,28 @@ public class DeploymentTest extends VertxTestBase {
await();
}
@Test
public void testCloseIsolationGroup() throws Exception {
boolean expectedSuccess = Thread.currentThread().getContextClassLoader() instanceof URLClassLoader;
String dir = createClassOutsideClasspath("MyVerticle");
List<String> extraClasspath = Arrays.asList(dir);
Vertx vertx = Vertx.vertx();
try {
DeploymentOptions options = new DeploymentOptions().setIsolationGroup("somegroup").setExtraClasspath(extraClasspath);
vertx.deployVerticle("java:" + ExtraCPVerticleNotInParentLoader.class.getCanonicalName(), options, onSuccess(id -> {
vertx.close(onSuccess(v -> {
assertTrue(ExtraCPVerticleNotInParentLoader.cl.isClosed());
testComplete();
}));
}));
assertTrue(expectedSuccess);
await();
} catch (IllegalStateException e) {
assertFalse(expectedSuccess);
} finally {
ExtraCPVerticleNotInParentLoader.cl = null;
}
}
public static class ParentVerticle extends AbstractVerticle {
@Override

View File

@@ -19,9 +19,12 @@ import org.junit.Assert;
* @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
*/
public class ExtraCPVerticleNotInParentLoader extends AbstractVerticle {
public static IsolatingClassLoader cl;
@Override
public void start() throws Exception {
IsolatingClassLoader cl = (IsolatingClassLoader) Thread.currentThread().getContextClassLoader();
cl = (IsolatingClassLoader) Thread.currentThread().getContextClassLoader();
Class extraCPClass = cl.loadClass("MyVerticle");
Assert.assertSame(extraCPClass.getClassLoader(), cl);
try {