From fa341c66e9826a7f6a80083e9c84490116607e73 Mon Sep 17 00:00:00 2001 From: Brett Meyer Date: Thu, 5 Sep 2013 18:47:30 -0400 Subject: [PATCH] HHH-8363 destroy the parent ServiceRegistry and stop its provided --- .../internal/ClassLoaderServiceImpl.java | 21 ++-- .../BootstrapServiceRegistryImpl.java | 27 +++++- .../internal/AbstractServiceRegistryImpl.java | 8 +- .../service/ClassLoaderServiceImplTest.java | 95 ++++++++++++++++++- .../test/service/TestIntegrator.java | 51 ++++++++++ .../org.hibernate.integrator.spi.Integrator | 1 + 6 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/service/TestIntegrator.java create mode 100644 hibernate-core/src/test/resources/org/hibernate/test/service/org.hibernate.integrator.spi.Integrator diff --git a/hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java index c1e15dfff3..a849d0d0c5 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java @@ -30,10 +30,10 @@ import java.util.ArrayList; 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; @@ -54,7 +54,7 @@ public class ClassLoaderServiceImpl implements ClassLoaderService { private final AggregatedClassLoader aggregatedClassLoader; - private final LinkedList serviceLoaders = new LinkedList(); + private final Map serviceLoaders = new HashMap(); /** * Constructs a ClassLoaderServiceImpl with standard set-up @@ -321,21 +321,28 @@ public class ClassLoaderServiceImpl implements ClassLoaderService { @Override public LinkedHashSet loadJavaServices(Class serviceContract) { - ServiceLoader serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader ); + ServiceLoader serviceLoader; + if ( serviceLoaders.containsKey( serviceContract ) ) { + serviceLoader = serviceLoaders.get( serviceContract ); + } + else { + serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader ); + serviceLoaders.put( serviceContract, serviceLoader ); + } + final LinkedHashSet services = new LinkedHashSet(); 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(); } // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! diff --git a/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/BootstrapServiceRegistryImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/BootstrapServiceRegistryImpl.java index 048733e3da..47542569d0 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/BootstrapServiceRegistryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/BootstrapServiceRegistryImpl.java @@ -28,6 +28,7 @@ import java.util.LinkedHashSet; 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.boot.registry.BootstrapServiceRegistry; import org.hibernate.service.Service; import org.hibernate.service.ServiceRegistry; @@ -35,10 +36,13 @@ import org.hibernate.boot.registry.classloading.internal.ClassLoaderServiceImpl; import org.hibernate.boot.registry.classloading.spi.ClassLoaderService; import org.hibernate.boot.registry.selector.internal.StrategySelectorImpl; import org.hibernate.boot.registry.selector.spi.StrategySelector; +import org.hibernate.service.internal.AbstractServiceRegistryImpl; import org.hibernate.service.spi.ServiceBinding; 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:
    @@ -56,6 +60,12 @@ import org.hibernate.service.spi.ServiceRegistryImplementor; */ public class BootstrapServiceRegistryImpl implements ServiceRegistryImplementor, BootstrapServiceRegistry, ServiceBinding.ServiceLifecycleOwner { + + private static final CoreMessageLogger LOG = Logger.getMessageLogger( + CoreMessageLogger.class, + BootstrapServiceRegistryImpl.class.getName() + ); + private static final LinkedHashSet NO_INTEGRATORS = new LinkedHashSet(); private final ServiceBinding classLoaderServiceBinding; @@ -170,6 +180,13 @@ public class BootstrapServiceRegistryImpl @Override public void destroy() { + destroy( classLoaderServiceBinding ); + destroy( strategySelectorBinding ); + destroy( integratorServiceBinding ); + } + + private void destroy(ServiceBinding serviceBinding) { + serviceBinding.getLifecycleOwner().stopService( serviceBinding ); } @Override @@ -199,7 +216,15 @@ public class BootstrapServiceRegistryImpl @Override public void stopService(ServiceBinding 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() ); + } + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java b/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java index 0b160a1636..aa3d772e67 100644 --- a/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java @@ -85,7 +85,11 @@ public abstract class AbstractServiceRegistryImpl @SuppressWarnings({ "unchecked" }) protected void createServiceBinding(ServiceInitiator 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 void createServiceBinding(ProvidedService providedService) { @@ -274,6 +278,8 @@ public abstract class AbstractServiceRegistryImpl serviceBindingList.clear(); } serviceBindingMap.clear(); + + parent.destroy(); } @Override diff --git a/hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java b/hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java index 7609a5ffc8..53a6d8f03b 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java @@ -1,13 +1,31 @@ package org.hibernate.test.service; -import org.hibernate.boot.registry.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 static org.junit.Assert.fail; -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 java.util.NoSuchElementException; + +import javax.persistence.Entity; + +import org.hibernate.boot.registry.BootstrapServiceRegistryBuilder; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.boot.registry.classloading.internal.ClassLoaderServiceImpl; +import org.hibernate.boot.registry.classloading.spi.ClassLoaderService; +import org.hibernate.integrator.spi.Integrator; +import org.hibernate.internal.util.ConfigHelper; +import org.hibernate.internal.util.ReflectHelper; +import org.hibernate.service.ServiceRegistry; +import org.hibernate.testing.TestForIssue; +import org.junit.Assert; +import org.junit.Test; /** * @author Artem V. Navrotskiy @@ -36,8 +54,77 @@ public class ClassLoaderServiceImplTest { 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 StandardServiceRegistryBuilder( bootstrapBuilder.build() ).build(); + final ClassLoaderService classLoaderService = serviceRegistry.getService( ClassLoaderService.class ); + + TestIntegrator testIntegrator1 = findTestIntegrator( classLoaderService ); + assertNotNull( testIntegrator1 ); + + TestIntegrator testIntegrator2 = findTestIntegrator( classLoaderService ); + assertNotNull( testIntegrator2 ); + + assertSame( testIntegrator1, testIntegrator2 ); + + StandardServiceRegistryBuilder.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 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 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() { + 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. * diff --git a/hibernate-core/src/test/java/org/hibernate/test/service/TestIntegrator.java b/hibernate-core/src/test/java/org/hibernate/test/service/TestIntegrator.java new file mode 100644 index 0000000000..cac748cd64 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/service/TestIntegrator.java @@ -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"); + } + +} diff --git a/hibernate-core/src/test/resources/org/hibernate/test/service/org.hibernate.integrator.spi.Integrator b/hibernate-core/src/test/resources/org/hibernate/test/service/org.hibernate.integrator.spi.Integrator new file mode 100644 index 0000000000..373ccfec43 --- /dev/null +++ b/hibernate-core/src/test/resources/org/hibernate/test/service/org.hibernate.integrator.spi.Integrator @@ -0,0 +1 @@ +org.hibernate.test.service.TestIntegrator \ No newline at end of file