HHH-8363 SessionFactoryServiceRegistryImpl should not call parent#destroy, test failures, formatting

Author: brmeyer

hibernate-core/src/main/java/org/hibernate/boot/registry/classloading/internal/ClassLoaderServiceImpl.java

@@ -343,14 +343,15 @@ public <S> LinkedHashSet<S> loadJavaServices(Class<S> serviceContract) {
 
     @Override
     public void stop() {
-		for (ServiceLoader serviceLoader : serviceLoaders.values()) {
+		for ( ServiceLoader serviceLoader : serviceLoaders.values() ) {
 			serviceLoader.reload(); // clear service loader providers
 		}
 		serviceLoaders.clear();
-        if (aggregatedClassLoader!=null){
-            aggregatedClassLoader.destroy();
-            aggregatedClassLoader = null;
-        }
+		
+		if ( aggregatedClassLoader != null ) {
+			aggregatedClassLoader.destroy();
+			aggregatedClassLoader = null;
+		}
     }
 
 	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

hibernate-core/src/main/java/org/hibernate/boot/registry/internal/StandardServiceRegistryImpl.java

@@ -26,15 +26,16 @@
 import java.util.List;
 import java.util.Map;
 
-import org.hibernate.boot.registry.StandardServiceInitiator;
 import org.hibernate.boot.registry.BootstrapServiceRegistry;
+import org.hibernate.boot.registry.StandardServiceInitiator;
 import org.hibernate.boot.registry.StandardServiceRegistry;
 import org.hibernate.service.Service;
 import org.hibernate.service.internal.AbstractServiceRegistryImpl;
 import org.hibernate.service.internal.ProvidedService;
 import org.hibernate.service.spi.Configurable;
 import org.hibernate.service.spi.ServiceBinding;
 import org.hibernate.service.spi.ServiceInitiator;
+import org.hibernate.service.spi.ServiceRegistryImplementor;
 
 /**
  * Standard Hibernate implementation of the standard service registry.
@@ -89,4 +90,11 @@ public <R extends Service> void configureService(ServiceBinding<R> serviceBindin
 		}
 	}
 
+	@Override
+	public void destroy() {
+		super.destroy();
+		if ( getParentServiceRegistry() != null ) {
+			( (ServiceRegistryImplementor) getParentServiceRegistry() ).destroy();
+		}
+	}
 }

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

@@ -87,9 +87,6 @@ public AbstractServiceRegistryImpl(BootstrapServiceRegistry bootstrapServiceRegi
 	protected <R extends Service> void createServiceBinding(ServiceInitiator<R> 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) {
@@ -278,10 +275,6 @@ public void destroy() {
 			serviceBindingList.clear();
 		}
 		serviceBindingMap.clear();
-
-        if (parent!=null){
-    		parent.destroy();
-        }
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/test/service/ClassLoaderServiceImplTest.java

@@ -1,17 +1,15 @@
 package org.hibernate.test.service;
 
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
-import static org.junit.Assert.fail;
 
 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;
 
@@ -21,7 +19,6 @@
 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;
@@ -58,6 +55,8 @@ public void testSystemClassLoaderNotOverriding() throws IOException, ClassNotFou
     /**
      * 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.
+     * 
+     * TODO: Is there a way to test that the ServiceLoader was actually reset?
      */
     @Test
     @TestForIssue(jiraKey = "HHH-8363")
@@ -77,11 +76,9 @@ public void testStoppableClassLoaderService() {
 
     	StandardServiceRegistryBuilder.destroy( serviceRegistry );
 
+    	// Should return null -- aggregratedClassLoader blown away.
     	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 );
+    	assertNull( testIntegrator2 );
     }
 
     private TestIntegrator findTestIntegrator(ClassLoaderService classLoaderService) {

hibernate-core/src/test/java/org/hibernate/test/service/TestIntegrator.java

@@ -34,18 +34,18 @@ 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-envers/src/main/java/org/hibernate/envers/configuration/spi/AuditConfiguration.java

@@ -186,12 +186,13 @@ public synchronized static AuditConfiguration getFor(Configuration cfg, ClassLoa
 		return verCfg;
 	}
 
-    public void destroy() {
-        for (Map.Entry<Configuration, AuditConfiguration> c: new HashSet<Map.Entry<Configuration, AuditConfiguration>>(CFGS.entrySet())){
-            if (c.getValue() == this){//this is nasty cleanup fix, whole static CFGS should be reworked
-                CFGS.remove(c.getKey());
-            }
-        }
-        classLoaderService = null;
-    }
+	public void destroy() {
+		for ( Map.Entry<Configuration, AuditConfiguration> c : new HashSet<Map.Entry<Configuration, AuditConfiguration>>(
+				CFGS.entrySet() ) ) {
+			if ( c.getValue() == this ) { // this is nasty cleanup fix, whole static CFGS should be reworked
+				CFGS.remove( c.getKey() );
+			}
+		}
+		classLoaderService = null;
+	}
 }