Hibernate
/
hibernate-orm
mirror of https://github.com/hibernate/hibernate-orm
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

HHH-10427 - Fix ServiceRegistry creates multiple service instances and returns uninitialized services 

(cherry picked from commit b74868d658)
This commit is contained in:
Gail Badner 2016-06-13 22:31:17 -07:00
parent 0951a62286
commit 967e96631f
3 changed files with 217 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -11,6 +11,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import org.hibernate.boot.registry.BootstrapServiceRegistry;
import org.hibernate.cfg.Environment;
@ -36,6 +37,7 @@ import org.hibernate.service.spi.Stoppable;
* Basic implementation of the ServiceRegistry and ServiceRegistryImplementor contracts
*
* @author Steve Ebersole
* @author Sanne Grinovero
*/
public abstract class AbstractServiceRegistryImpl
implements ServiceRegistryImplementor, ServiceBinding.ServiceLifecycleOwner {
@ -48,16 +50,24 @@ public abstract class AbstractServiceRegistryImpl
private final boolean allowCrawling;
private final ConcurrentServiceBinding<Class,ServiceBinding> serviceBindingMap = new ConcurrentServiceBinding<Class,ServiceBinding>();
private ConcurrentServiceBinding<Class,Class> roleXref;
private final ConcurrentServiceBinding<Class,Class> roleXref = new ConcurrentServiceBinding<Class,Class>();
// The services stored in initializedServiceByRole are completely initialized
// (i.e., configured, dependencies injected, and started)
private final ConcurrentServiceBinding<Class,Service> initializedServiceByRole = new ConcurrentServiceBinding<Class, Service>();
// IMPL NOTE : the list used for ordered destruction. Cannot used map above because we need to
// iterate it in reverse order which is only available through ListIterator
// assume 20 services for initial sizing
// All access guarded by synchronization on the serviceBindingList itself.
private final List<ServiceBinding> serviceBindingList = CollectionHelper.arrayList( 20 );
// Guarded by synchronization on this.
private boolean autoCloseRegistry;
// Guarded by synchronization on this.
private Set<ServiceRegistryImplementor> childRegistries;
private final AtomicBoolean active = new AtomicBoolean( true );
@SuppressWarnings( {"UnusedDeclaration"})
protected AbstractServiceRegistryImpl() {
this( (ServiceRegistryImplementor) null );
@ -143,11 +153,9 @@ public abstract class AbstractServiceRegistryImpl
}
// look for a previously resolved alternate registration
if ( roleXref != null ) {
final Class alternative = roleXref.get( serviceRole );
if ( alternative != null ) {
return serviceBindingMap.get( alternative );
}
final Class alternative = roleXref.get( serviceRole );
if ( alternative != null ) {
return serviceBindingMap.get( alternative );
}
// perform a crawl looking for an alternate registration
@ -171,25 +179,37 @@ public abstract class AbstractServiceRegistryImpl
}
private void registerAlternate(Class alternate, Class target) {
if ( roleXref == null ) {
roleXref = new ConcurrentServiceBinding<Class,Class>();
}
roleXref.put( alternate, target );
}
@Override
public <R extends Service> R getService(Class<R> serviceRole) {
final ServiceBinding<R> serviceBinding = locateServiceBinding( serviceRole );
if ( serviceBinding == null ) {
throw new UnknownServiceException( serviceRole );
// TODO: should an exception be thrown if active == false???
R service = serviceRole.cast( initializedServiceByRole.get( serviceRole ) );
if ( service != null ) {
return service;
}
R service = serviceBinding.getService();
if ( service == null ) {
service = initializeService( serviceBinding );
}
//Any service initialization needs synchronization
synchronized ( this ) {
// Check again after having acquired the lock:
service = serviceRole.cast( initializedServiceByRole.get( serviceRole ) );
if ( service != null ) {
return service;
}
return service;
final ServiceBinding<R> serviceBinding = locateServiceBinding( serviceRole );
if ( serviceBinding == null ) {
throw new UnknownServiceException( serviceRole );
}
service = serviceBinding.getService();
if ( service == null ) {
service = initializeService( serviceBinding );
}
// add the service only after it is completely initialized
initializedServiceByRole.put( serviceRole, service );
return service;
}
}
protected <R extends Service> void registerService(ServiceBinding<R> serviceBinding, R service) {
@ -320,35 +340,33 @@ public abstract class AbstractServiceRegistryImpl
}
}
private boolean active = true;
public boolean isActive() {
return active;
return active.get();
}
@Override
@SuppressWarnings( {"unchecked"})
public void destroy() {
if ( !active ) {
return;
}
active = false;
try {
synchronized (serviceBindingList) {
ListIterator<ServiceBinding> serviceBindingsIterator = serviceBindingList.listIterator(
serviceBindingList.size()
);
while ( serviceBindingsIterator.hasPrevious() ) {
final ServiceBinding serviceBinding = serviceBindingsIterator.previous();
serviceBinding.getLifecycleOwner().stopService( serviceBinding );
public synchronized void destroy() {
if ( active.compareAndSet( true, false ) ) {
try {
//First thing, make sure that the fast path read is disabled so that
//threads not owning the synchronization lock can't get an invalid Service:
initializedServiceByRole.clear();
synchronized (serviceBindingList) {
ListIterator<ServiceBinding> serviceBindingsIterator = serviceBindingList.listIterator(
serviceBindingList.size()
);
while ( serviceBindingsIterator.hasPrevious() ) {
final ServiceBinding serviceBinding = serviceBindingsIterator.previous();
serviceBinding.getLifecycleOwner().stopService( serviceBinding );
}
serviceBindingList.clear();
}
serviceBindingList.clear();
serviceBindingMap.clear();
}
finally {
parent.deRegisterChild( this );
}
serviceBindingMap.clear();
}
finally {
parent.deRegisterChild( this );
}
}
@ -366,7 +384,7 @@ public abstract class AbstractServiceRegistryImpl
}
@Override
public void registerChild(ServiceRegistryImplementor child) {
public synchronized void registerChild(ServiceRegistryImplementor child) {
if ( childRegistries == null ) {
childRegistries = new HashSet<ServiceRegistryImplementor>();
}
@ -379,7 +397,7 @@ public abstract class AbstractServiceRegistryImpl
}
@Override
public void deRegisterChild(ServiceRegistryImplementor child) {
public synchronized void deRegisterChild(ServiceRegistryImplementor child) {
if ( childRegistries == null ) {
throw new IllegalStateException( "No child ServiceRegistry registrations found" );
}

2
hibernate-core/src/main/java/org/hibernate/service/spi/ServiceBinding.java
View File

@ -31,7 +31,7 @@ public final class ServiceBinding<R extends Service> {
private final ServiceLifecycleOwner lifecycleOwner;
private final Class<R> serviceRole;
private final ServiceInitiator<R> serviceInitiator;
private R service;
private volatile R service;
public ServiceBinding(ServiceLifecycleOwner lifecycleOwner, Class<R> serviceRole, R service) {
this.lifecycleOwner = lifecycleOwner;

157
hibernate-core/src/test/java/org/hibernate/service/ServiceRegistryTest.java Normal file
View File

@ -0,0 +1,157 @@
package org.hibernate.service;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
import org.hibernate.boot.registry.StandardServiceInitiator;
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
import org.hibernate.service.spi.Configurable;
import org.hibernate.service.spi.ServiceRegistryAwareService;
import org.hibernate.service.spi.ServiceRegistryImplementor;
import org.hibernate.service.spi.Startable;
import org.junit.Test;
import org.hibernate.testing.TestForIssue;
import static org.junit.Assert.assertTrue;
@TestForIssue(jiraKey = "HHH-10427")
public class ServiceRegistryTest {
private final ServiceRegistry registry = buildRegistry();
private final static int NUMBER_OF_THREADS = 100;
private StandardServiceRegistryBuilder standardServiceRegistryBuilder;
@Test
public void testOnlyOneInstanceOfTheServiceShouldBeCreated() throws InterruptedException, ExecutionException {
Future<SlowInitializationService>[] serviceIdentities = execute();
SlowInitializationService previousResult = null;
for ( Future<SlowInitializationService> future : serviceIdentities ) {
final SlowInitializationService result = future.get();
if ( previousResult == null ) {
previousResult = result;
}
else {
assertTrue( "There are more than one instance of the service", result == previousResult );
}
}
standardServiceRegistryBuilder.destroy( registry );
}
private ServiceRegistry buildRegistry() {
standardServiceRegistryBuilder = new StandardServiceRegistryBuilder();
return standardServiceRegistryBuilder.addInitiator( new SlowServiceInitiator() ).build();
}
private FutureTask<SlowInitializationService>[] execute()
throws InterruptedException, ExecutionException {
FutureTask<SlowInitializationService>[] results = new FutureTask[NUMBER_OF_THREADS];
ExecutorService executor = Executors.newFixedThreadPool( NUMBER_OF_THREADS );
for ( int i = 0; i < NUMBER_OF_THREADS; i++ ) {
results[i] = new FutureTask<>( new ServiceCallable( registry ) );
executor.execute( results[i] );
}
return results;
}
public class ServiceCallable implements Callable<SlowInitializationService> {
private final ServiceRegistry registry;
public ServiceCallable(ServiceRegistry registry) {
this.registry = registry;
}
@Override
public SlowInitializationService call() throws Exception {
final SlowInitializationService service = registry.getService( SlowInitializationService.class );
assertTrue( "The service is not initialized", service.isInitialized() );
assertTrue( "The service is not configured", service.isConfigured() );
assertTrue( "The service is not started", service.isStarted() );
return service;
}
}
public class SlowInitializationService implements ServiceRegistryAwareService, Configurable, Startable, Service {
private final static int TIME_TO_SLEEP = 100;
private boolean initialized;
private boolean configured;
private boolean started;
public SlowInitializationService() {
try {
Thread.sleep( TIME_TO_SLEEP );
}
catch (InterruptedException e) {
}
}
@Override
public void injectServices(ServiceRegistryImplementor serviceRegistry) {
try {
Thread.sleep( TIME_TO_SLEEP );
}
catch (InterruptedException e) {
}
initialized = true;
}
@Override
public void configure(Map configurationValues) {
try {
Thread.sleep( TIME_TO_SLEEP );
}
catch (InterruptedException e) {
}
configured = true;
}
@Override
public void start() {
try {
Thread.sleep( TIME_TO_SLEEP );
}
catch (InterruptedException e) {
}
started = true;
}
public boolean isInitialized() {
return initialized;
}
public boolean isConfigured() {
return configured;
}
public boolean isStarted() {
return started;
}
}
public class SlowServiceInitiator implements StandardServiceInitiator<SlowInitializationService> {
@Override
public Class<SlowInitializationService> getServiceInitiated() {
return SlowInitializationService.class;
}
@Override
public SlowInitializationService initiateService(Map configurationValues, ServiceRegistryImplementor registry) {
return new SlowInitializationService();
}
}
}