Skip to content

Commit

Permalink
HHH-8363 destroy the parent ServiceRegistry and stop its provided
Browse files Browse the repository at this point in the history
Conflicts:
	hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java
	hibernate-core/src/main/java/org/hibernate/boot/registry/internal/BootstrapServiceRegistryImpl.java
	hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java
  • Loading branch information
brmeyer committed Sep 17, 2013
1 parent db6841e commit a89e88f
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 13 deletions.
Expand Up @@ -30,10 +30,10 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.ServiceLoader;
Expand All @@ -54,7 +54,7 @@ public class ClassLoaderServiceImpl implements ClassLoaderService {

private final ClassLoader aggregatedClassLoader;

private final LinkedList<ServiceLoader> serviceLoaders = new LinkedList<ServiceLoader>();
private final Map<Class, ServiceLoader> serviceLoaders = new HashMap<Class, ServiceLoader>();

public ClassLoaderServiceImpl() {
this( ClassLoaderServiceImpl.class.getClassLoader() );
Expand Down Expand Up @@ -234,21 +234,28 @@ public List<URL> locateResources(String name) {

@Override
public <S> LinkedHashSet<S> loadJavaServices(Class<S> serviceContract) {
ServiceLoader<S> serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader );
ServiceLoader<S> serviceLoader;
if ( serviceLoaders.containsKey( serviceContract ) ) {
serviceLoader = serviceLoaders.get( serviceContract );
}
else {
serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader );
serviceLoaders.put( serviceContract, serviceLoader );
}

final LinkedHashSet<S> services = new LinkedHashSet<S>();
for ( S service : serviceLoader ) {
services.add( service );
}
serviceLoaders.add( serviceLoader );
return services;
}

@Override
public void stop() {
while ( !serviceLoaders.isEmpty() ) {
ServiceLoader loader = serviceLoaders.removeLast();
loader.reload(); // clear service loader providers
for (ServiceLoader serviceLoader : serviceLoaders.values()) {
serviceLoader.reload(); // clear service loader providers
}
serviceLoaders.clear();
}

private static class AggregatedClassLoader extends ClassLoader {
Expand Down
Expand Up @@ -85,7 +85,11 @@ public AbstractServiceRegistryImpl(BootstrapServiceRegistry bootstrapServiceRegi

@SuppressWarnings({ "unchecked" })
protected <R extends Service> void createServiceBinding(ServiceInitiator<R> initiator) {
serviceBindingMap.put( initiator.getServiceInitiated(), new ServiceBinding( this, initiator ) );
final ServiceBinding serviceBinding = new ServiceBinding( this, initiator );
serviceBindingMap.put( initiator.getServiceInitiated(), serviceBinding );
synchronized ( serviceBindingList ) {
serviceBindingList.add( serviceBinding );
}
}

protected <R extends Service> void createServiceBinding(ProvidedService<R> providedService) {
Expand Down Expand Up @@ -274,6 +278,8 @@ public void destroy() {
serviceBindingList.clear();
}
serviceBindingMap.clear();

parent.destroy();
}

@Override
Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.hibernate.integrator.internal.IntegratorServiceImpl;
import org.hibernate.integrator.spi.Integrator;
import org.hibernate.integrator.spi.IntegratorService;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.service.BootstrapServiceRegistry;
import org.hibernate.service.Service;
import org.hibernate.service.ServiceRegistry;
Expand All @@ -37,6 +38,8 @@
import org.hibernate.service.spi.ServiceException;
import org.hibernate.service.spi.ServiceInitiator;
import org.hibernate.service.spi.ServiceRegistryImplementor;
import org.hibernate.service.spi.Stoppable;
import org.jboss.logging.Logger;

/**
* {@link ServiceRegistry} implementation containing specialized "bootstrap" services, specifically:<ul>
Expand All @@ -48,6 +51,12 @@
*/
public class BootstrapServiceRegistryImpl
implements ServiceRegistryImplementor, BootstrapServiceRegistry, ServiceBinding.ServiceLifecycleOwner {

private static final CoreMessageLogger LOG = Logger.getMessageLogger(
CoreMessageLogger.class,
BootstrapServiceRegistryImpl.class.getName()
);

private static final LinkedHashSet<Integrator> NO_INTEGRATORS = new LinkedHashSet<Integrator>();

private final ServiceBinding<ClassLoaderService> classLoaderServiceBinding;
Expand Down Expand Up @@ -103,6 +112,12 @@ else if ( IntegratorService.class.equals( serviceRole ) ) {

@Override
public void destroy() {
destroy( classLoaderServiceBinding );
destroy( integratorServiceBinding );
}

private void destroy(ServiceBinding serviceBinding) {
serviceBinding.getLifecycleOwner().stopService( serviceBinding );
}

@Override
Expand Down Expand Up @@ -132,7 +147,15 @@ public <R extends Service> void startService(ServiceBinding<R> binding) {

@Override
public <R extends Service> void stopService(ServiceBinding<R> binding) {
throw new ServiceException( "Boot-strap registry should only contain provided services" );
final Service service = binding.getService();
if ( Stoppable.class.isInstance( service ) ) {
try {
( (Stoppable) service ).stop();
}
catch ( Exception e ) {
LOG.unableToStopService( service.getClass(), e.toString() );
}
}
}

}
@@ -1,13 +1,28 @@
package org.hibernate.test.service;

import org.hibernate.service.classloading.internal.ClassLoaderServiceImpl;
import org.junit.Assert;
import org.junit.Test;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;

import javax.persistence.Entity;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.Enumeration;
import java.util.LinkedHashSet;

import javax.persistence.Entity;

import org.hibernate.integrator.spi.Integrator;
import org.hibernate.internal.util.ConfigHelper;
import org.hibernate.service.BootstrapServiceRegistryBuilder;
import org.hibernate.service.ServiceRegistry;
import org.hibernate.service.ServiceRegistryBuilder;
import org.hibernate.service.classloading.internal.ClassLoaderServiceImpl;
import org.hibernate.service.classloading.spi.ClassLoaderService;
import org.hibernate.testing.TestForIssue;
import org.junit.Assert;
import org.junit.Test;

/**
* @author Artem V. Navrotskiy
Expand Down Expand Up @@ -36,8 +51,77 @@ public void testSystemClassLoaderNotOverriding() throws IOException, ClassNotFou
Assert.assertSame("Should not return class loaded from the parent classloader of ClassLoaderServiceImpl",
objectClass, anotherClass);
}

/**
* HHH-8363 discovered multiple leaks within CLS. Most notably, it wasn't getting GC'd due to holding
* references to ServiceLoaders. Ensure that the addition of Stoppable functionality cleans up properly.
*/
@Test
@TestForIssue(jiraKey = "HHH-8363")
public void testStoppableClassLoaderService() {
final BootstrapServiceRegistryBuilder bootstrapBuilder = new BootstrapServiceRegistryBuilder();
bootstrapBuilder.with( new TestClassLoader() );
final ServiceRegistry serviceRegistry = new ServiceRegistryBuilder( bootstrapBuilder.build() ).buildServiceRegistry();
final ClassLoaderService classLoaderService = serviceRegistry.getService( ClassLoaderService.class );

TestIntegrator testIntegrator1 = findTestIntegrator( classLoaderService );
assertNotNull( testIntegrator1 );

TestIntegrator testIntegrator2 = findTestIntegrator( classLoaderService );
assertNotNull( testIntegrator2 );

assertSame( testIntegrator1, testIntegrator2 );

ServiceRegistryBuilder.destroy( serviceRegistry );

testIntegrator2 = findTestIntegrator( classLoaderService );
assertNotNull( testIntegrator2 );

// destroy should have cleared the ServiceLoader caches, forcing the services to be re-created when called upon
assertNotSame( testIntegrator1, testIntegrator2 );
}

private TestIntegrator findTestIntegrator(ClassLoaderService classLoaderService) {
final LinkedHashSet<Integrator> integrators = classLoaderService.loadJavaServices( Integrator.class );
for (Integrator integrator : integrators) {
if (integrator instanceof TestIntegrator) {
return (TestIntegrator) integrator;
}
}
return null;
}

private static class TestClassLoader extends ClassLoader {

/**
* testStoppableClassLoaderService() needs a custom JDK service implementation. Rather than using a real one
* on the test classpath, force it in here.
*/
@Override
protected Enumeration<URL> findResources(String name) throws IOException {
if (name.equals( "META-INF/services/org.hibernate.integrator.spi.Integrator" )) {
final URL serviceUrl = ConfigHelper.findAsResource(
"org/hibernate/test/service/org.hibernate.integrator.spi.Integrator" );
return new Enumeration<URL>() {
boolean hasMore = true;

@Override
public boolean hasMoreElements() {
return hasMore;
}

@Override
public URL nextElement() {
hasMore = false;
return serviceUrl;
}
};
}
else {
return java.util.Collections.emptyEnumeration();
}
}

/**
* Reloading class from binary file.
*
Expand Down
@@ -0,0 +1,51 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* JBoss, Home of Professional Open Source
* Copyright 2013 Red Hat Inc. and/or its affiliates and other contributors
* as indicated by the @authors tag. All rights reserved.
* See the copyright.txt in the distribution for a
* full listing of individual contributors.
*
* This copyrighted material is made available to anyone wishing to use,
* modify, copy, or redistribute it subject to the terms and conditions
* of the GNU Lesser General Public License, v. 2.1.
* This program is distributed in the hope that it will be useful, but WITHOUT A
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
* PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public License,
* v.2.1 along with this distribution; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.hibernate.test.service;

import org.hibernate.cfg.Configuration;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.integrator.spi.Integrator;
import org.hibernate.metamodel.source.MetadataImplementor;
import org.hibernate.service.spi.SessionFactoryServiceRegistry;

/**
* @author Brett Meyer
*/
public class TestIntegrator implements Integrator {

@Override
public void integrate(Configuration configuration, SessionFactoryImplementor sessionFactory,
SessionFactoryServiceRegistry serviceRegistry) {
System.out.println("foo");
}

@Override
public void integrate(MetadataImplementor metadata, SessionFactoryImplementor sessionFactory,
SessionFactoryServiceRegistry serviceRegistry) {
System.out.println("foo");
}

@Override
public void disintegrate(SessionFactoryImplementor sessionFactory, SessionFactoryServiceRegistry serviceRegistry) {
System.out.println("foo");
}

}
@@ -0,0 +1 @@
org.hibernate.test.service.TestIntegrator

0 comments on commit a89e88f

Please sign in to comment.