Skip to content

Commit fa341c6

Browse files
brmeyerbrmeyer
committed
HHH-8363 destroy the parent ServiceRegistry and stop its provided
1 parent 3fdffcb commit fa341c6

File tree

6 files changed

+190
-13
lines changed

6 files changed

+190
-13
lines changed

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@
3030
import java.util.Collection;
3131
import java.util.Collections;
3232
import java.util.Enumeration;
33+
import java.util.HashMap;
3334
import java.util.HashSet;
3435
import java.util.Iterator;
3536
import java.util.LinkedHashSet;
36-
import java.util.LinkedList;
3737
import java.util.List;
3838
import java.util.Map;
3939
import java.util.ServiceLoader;
@@ -54,7 +54,7 @@ public class ClassLoaderServiceImpl implements ClassLoaderService {
5454

5555
private final AggregatedClassLoader aggregatedClassLoader;
5656

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

5959
/**
6060
* Constructs a ClassLoaderServiceImpl with standard set-up
@@ -321,21 +321,28 @@ public List<URL> locateResources(String name) {
321321

322322
@Override
323323
public <S> LinkedHashSet<S> loadJavaServices(Class<S> serviceContract) {
324-
ServiceLoader<S> serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader );
324+
ServiceLoader<S> serviceLoader;
325+
if ( serviceLoaders.containsKey( serviceContract ) ) {
326+
serviceLoader = serviceLoaders.get( serviceContract );
327+
}
328+
else {
329+
serviceLoader = ServiceLoader.load( serviceContract, aggregatedClassLoader );
330+
serviceLoaders.put( serviceContract, serviceLoader );
331+
}
332+
325333
final LinkedHashSet<S> services = new LinkedHashSet<S>();
326334
for ( S service : serviceLoader ) {
327335
services.add( service );
328336
}
329-
serviceLoaders.add( serviceLoader );
330337
return services;
331338
}
332339

333340
@Override
334341
public void stop() {
335-
while ( !serviceLoaders.isEmpty() ) {
336-
ServiceLoader loader = serviceLoaders.removeLast();
337-
loader.reload(); // clear service loader providers
342+
for (ServiceLoader serviceLoader : serviceLoaders.values()) {
343+
serviceLoader.reload(); // clear service loader providers
338344
}
345+
serviceLoaders.clear();
339346
}
340347

341348
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,21 @@
2828
import org.hibernate.integrator.internal.IntegratorServiceImpl;
2929
import org.hibernate.integrator.spi.Integrator;
3030
import org.hibernate.integrator.spi.IntegratorService;
31+
import org.hibernate.internal.CoreMessageLogger;
3132
import org.hibernate.boot.registry.BootstrapServiceRegistry;
3233
import org.hibernate.service.Service;
3334
import org.hibernate.service.ServiceRegistry;
3435
import org.hibernate.boot.registry.classloading.internal.ClassLoaderServiceImpl;
3536
import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
3637
import org.hibernate.boot.registry.selector.internal.StrategySelectorImpl;
3738
import org.hibernate.boot.registry.selector.spi.StrategySelector;
39+
import org.hibernate.service.internal.AbstractServiceRegistryImpl;
3840
import org.hibernate.service.spi.ServiceBinding;
3941
import org.hibernate.service.spi.ServiceException;
4042
import org.hibernate.service.spi.ServiceInitiator;
4143
import org.hibernate.service.spi.ServiceRegistryImplementor;
44+
import org.hibernate.service.spi.Stoppable;
45+
import org.jboss.logging.Logger;
4246

4347
/**
4448
* {@link ServiceRegistry} implementation containing specialized "bootstrap" services, specifically:<ul>
@@ -56,6 +60,12 @@
5660
*/
5761
public class BootstrapServiceRegistryImpl
5862
implements ServiceRegistryImplementor, BootstrapServiceRegistry, ServiceBinding.ServiceLifecycleOwner {
63+
64+
private static final CoreMessageLogger LOG = Logger.getMessageLogger(
65+
CoreMessageLogger.class,
66+
BootstrapServiceRegistryImpl.class.getName()
67+
);
68+
5969
private static final LinkedHashSet<Integrator> NO_INTEGRATORS = new LinkedHashSet<Integrator>();
6070

6171
private final ServiceBinding<ClassLoaderService> classLoaderServiceBinding;
@@ -170,6 +180,13 @@ else if ( IntegratorService.class.equals( serviceRole ) ) {
170180

171181
@Override
172182
public void destroy() {
183+
destroy( classLoaderServiceBinding );
184+
destroy( strategySelectorBinding );
185+
destroy( integratorServiceBinding );
186+
}
187+
188+
private void destroy(ServiceBinding serviceBinding) {
189+
serviceBinding.getLifecycleOwner().stopService( serviceBinding );
173190
}
174191

175192
@Override
@@ -199,7 +216,15 @@ public <R extends Service> void startService(ServiceBinding<R> binding) {
199216

200217
@Override
201218
public <R extends Service> void stopService(ServiceBinding<R> binding) {
202-
throw new ServiceException( "Boot-strap registry should only contain provided services" );
219+
final Service service = binding.getService();
220+
if ( Stoppable.class.isInstance( service ) ) {
221+
try {
222+
( (Stoppable) service ).stop();
223+
}
224+
catch ( Exception e ) {
225+
LOG.unableToStopService( service.getClass(), e.toString() );
226+
}
227+
}
203228
}
204229

205230
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,11 @@ public AbstractServiceRegistryImpl(BootstrapServiceRegistry bootstrapServiceRegi
8585

8686
@SuppressWarnings({ "unchecked" })
8787
protected <R extends Service> void createServiceBinding(ServiceInitiator<R> initiator) {
88-
serviceBindingMap.put( initiator.getServiceInitiated(), new ServiceBinding( this, initiator ) );
88+
final ServiceBinding serviceBinding = new ServiceBinding( this, initiator );
89+
serviceBindingMap.put( initiator.getServiceInitiated(), serviceBinding );
90+
synchronized ( serviceBindingList ) {
91+
serviceBindingList.add( serviceBinding );
92+
}
8993
}
9094

9195
protected <R extends Service> void createServiceBinding(ProvidedService<R> providedService) {
@@ -274,6 +278,8 @@ public void destroy() {
274278
serviceBindingList.clear();
275279
}
276280
serviceBindingMap.clear();
281+
282+
parent.destroy();
277283
}
278284

279285
@Override

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

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,31 @@
11
package org.hibernate.test.service;
22

3-
import org.hibernate.boot.registry.classloading.internal.ClassLoaderServiceImpl;
4-
import org.junit.Assert;
5-
import org.junit.Test;
3+
import static org.junit.Assert.assertNotNull;
4+
import static org.junit.Assert.assertNotSame;
5+
import static org.junit.Assert.assertSame;
6+
import static org.junit.Assert.fail;
67

7-
import javax.persistence.Entity;
88
import java.io.ByteArrayOutputStream;
99
import java.io.IOException;
1010
import java.io.InputStream;
11+
import java.net.URL;
12+
import java.util.Enumeration;
13+
import java.util.LinkedHashSet;
14+
import java.util.NoSuchElementException;
15+
16+
import javax.persistence.Entity;
17+
18+
import org.hibernate.boot.registry.BootstrapServiceRegistryBuilder;
19+
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
20+
import org.hibernate.boot.registry.classloading.internal.ClassLoaderServiceImpl;
21+
import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
22+
import org.hibernate.integrator.spi.Integrator;
23+
import org.hibernate.internal.util.ConfigHelper;
24+
import org.hibernate.internal.util.ReflectHelper;
25+
import org.hibernate.service.ServiceRegistry;
26+
import org.hibernate.testing.TestForIssue;
27+
import org.junit.Assert;
28+
import org.junit.Test;
1129

1230
/**
1331
* @author Artem V. Navrotskiy
@@ -36,8 +54,77 @@ public void testSystemClassLoaderNotOverriding() throws IOException, ClassNotFou
3654
Assert.assertSame("Should not return class loaded from the parent classloader of ClassLoaderServiceImpl",
3755
objectClass, anotherClass);
3856
}
57+
58+
/**
59+
* HHH-8363 discovered multiple leaks within CLS. Most notably, it wasn't getting GC'd due to holding
60+
* references to ServiceLoaders. Ensure that the addition of Stoppable functionality cleans up properly.
61+
*/
62+
@Test
63+
@TestForIssue(jiraKey = "HHH-8363")
64+
public void testStoppableClassLoaderService() {
65+
final BootstrapServiceRegistryBuilder bootstrapBuilder = new BootstrapServiceRegistryBuilder();
66+
bootstrapBuilder.with( new TestClassLoader() );
67+
final ServiceRegistry serviceRegistry = new StandardServiceRegistryBuilder( bootstrapBuilder.build() ).build();
68+
final ClassLoaderService classLoaderService = serviceRegistry.getService( ClassLoaderService.class );
69+
70+
TestIntegrator testIntegrator1 = findTestIntegrator( classLoaderService );
71+
assertNotNull( testIntegrator1 );
72+
73+
TestIntegrator testIntegrator2 = findTestIntegrator( classLoaderService );
74+
assertNotNull( testIntegrator2 );
75+
76+
assertSame( testIntegrator1, testIntegrator2 );
77+
78+
StandardServiceRegistryBuilder.destroy( serviceRegistry );
79+
80+
testIntegrator2 = findTestIntegrator( classLoaderService );
81+
assertNotNull( testIntegrator2 );
82+
83+
// destroy should have cleared the ServiceLoader caches, forcing the services to be re-created when called upon
84+
assertNotSame( testIntegrator1, testIntegrator2 );
85+
}
86+
87+
private TestIntegrator findTestIntegrator(ClassLoaderService classLoaderService) {
88+
final LinkedHashSet<Integrator> integrators = classLoaderService.loadJavaServices( Integrator.class );
89+
for (Integrator integrator : integrators) {
90+
if (integrator instanceof TestIntegrator) {
91+
return (TestIntegrator) integrator;
92+
}
93+
}
94+
return null;
95+
}
3996

4097
private static class TestClassLoader extends ClassLoader {
98+
99+
/**
100+
* testStoppableClassLoaderService() needs a custom JDK service implementation. Rather than using a real one
101+
* on the test classpath, force it in here.
102+
*/
103+
@Override
104+
protected Enumeration<URL> findResources(String name) throws IOException {
105+
if (name.equals( "META-INF/services/org.hibernate.integrator.spi.Integrator" )) {
106+
final URL serviceUrl = ConfigHelper.findAsResource(
107+
"org/hibernate/test/service/org.hibernate.integrator.spi.Integrator" );
108+
return new Enumeration<URL>() {
109+
boolean hasMore = true;
110+
111+
@Override
112+
public boolean hasMoreElements() {
113+
return hasMore;
114+
}
115+
116+
@Override
117+
public URL nextElement() {
118+
hasMore = false;
119+
return serviceUrl;
120+
}
121+
};
122+
}
123+
else {
124+
return java.util.Collections.emptyEnumeration();
125+
}
126+
}
127+
41128
/**
42129
* Reloading class from binary file.
43130
*

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* JBoss, Home of Professional Open Source
5+
* Copyright 2013 Red Hat Inc. and/or its affiliates and other contributors
6+
* as indicated by the @authors tag. All rights reserved.
7+
* See the copyright.txt in the distribution for a
8+
* full listing of individual contributors.
9+
*
10+
* This copyrighted material is made available to anyone wishing to use,
11+
* modify, copy, or redistribute it subject to the terms and conditions
12+
* of the GNU Lesser General Public License, v. 2.1.
13+
* This program is distributed in the hope that it will be useful, but WITHOUT A
14+
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
15+
* PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
16+
* You should have received a copy of the GNU Lesser General Public License,
17+
* v.2.1 along with this distribution; if not, write to the Free Software
18+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
19+
* MA 02110-1301, USA.
20+
*/
21+
package org.hibernate.test.service;
22+
23+
import org.hibernate.cfg.Configuration;
24+
import org.hibernate.engine.spi.SessionFactoryImplementor;
25+
import org.hibernate.integrator.spi.Integrator;
26+
import org.hibernate.metamodel.source.MetadataImplementor;
27+
import org.hibernate.service.spi.SessionFactoryServiceRegistry;
28+
29+
/**
30+
* @author Brett Meyer
31+
*/
32+
public class TestIntegrator implements Integrator {
33+
34+
@Override
35+
public void integrate(Configuration configuration, SessionFactoryImplementor sessionFactory,
36+
SessionFactoryServiceRegistry serviceRegistry) {
37+
System.out.println("foo");
38+
}
39+
40+
@Override
41+
public void integrate(MetadataImplementor metadata, SessionFactoryImplementor sessionFactory,
42+
SessionFactoryServiceRegistry serviceRegistry) {
43+
System.out.println("foo");
44+
}
45+
46+
@Override
47+
public void disintegrate(SessionFactoryImplementor sessionFactory, SessionFactoryServiceRegistry serviceRegistry) {
48+
System.out.println("foo");
49+
}
50+
51+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
org.hibernate.test.service.TestIntegrator

0 commit comments

Comments
 (0)