HHH-8363 destroy the parent ServiceRegistry and stop its provided

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<ServiceLoader> serviceLoaders = new LinkedList<ServiceLoader>();
+	private final Map<Class, ServiceLoader> serviceLoaders = new HashMap<Class, ServiceLoader>();
 
 	/**
 	 * Constructs a ClassLoaderServiceImpl with standard set-up
@@ -321,21 +321,28 @@ public class ClassLoaderServiceImpl implements ClassLoaderService {
 
 	@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();
     }
 
 	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

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:<ul>
@@ -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<Integrator> NO_INTEGRATORS = new LinkedHashSet<Integrator>();
 
 	private final ServiceBinding<ClassLoaderService> 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 <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() );
+			}
+		}
 	}
 
 }

hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java

@@ -85,7 +85,11 @@ public abstract class AbstractServiceRegistryImpl
 
 	@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) {
@@ -274,6 +278,8 @@ public abstract class AbstractServiceRegistryImpl
 			serviceBindingList.clear();
 		}
 		serviceBindingMap.clear();
+		
+		parent.destroy();
 	}
 
 	@Override

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<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.
          *

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");
+	}
+
+}

hibernate-core/src/test/resources/org/hibernate/test/service/org.hibernate.integrator.spi.Integrator

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