diff --git a/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/StandardServiceRegistryImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/StandardServiceRegistryImpl.java index eae8a15480..6662a774b1 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/StandardServiceRegistryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/registry/internal/StandardServiceRegistryImpl.java @@ -109,36 +109,56 @@ public class StandardServiceRegistryImpl extends AbstractServiceRegistryImpl imp * Not intended for general use. We need the ability to stop and "reactivate" a registry to allow * experimentation with technologies such as GraalVM, Quarkus and Cri-O. */ - public synchronized void resetAndReactivate(BootstrapServiceRegistry bootstrapServiceRegistry, + public void resetAndReactivate(BootstrapServiceRegistry bootstrapServiceRegistry, List> serviceInitiators, List> providedServices, Map configurationValues) { - if ( super.isActive() ) { - throw new IllegalStateException( "Can't reactivate an active registry" ); + thisLock.lock(); + try { + if ( super.isActive() ) { + throw new IllegalStateException( "Can't reactivate an active registry" ); + } + super.resetParent( bootstrapServiceRegistry ); + this.configurationValues = new HashMap( configurationValues ); + super.reactivate(); + applyServiceRegistrations( serviceInitiators, providedServices ); + } finally { + thisLock.unlock(); } - super.resetParent( bootstrapServiceRegistry ); - this.configurationValues = new HashMap( configurationValues ); - super.reactivate(); - applyServiceRegistrations( serviceInitiators, providedServices ); } @Override - public synchronized R initiateService(ServiceInitiator serviceInitiator) { - // todo : add check/error for unexpected initiator types? - return ( (StandardServiceInitiator) serviceInitiator ).initiateService( configurationValues, this ); - } - - @Override - public synchronized void configureService(ServiceBinding serviceBinding) { - if ( serviceBinding.getService() instanceof Configurable ) { - ( (Configurable) serviceBinding.getService() ).configure( configurationValues ); + public R initiateService(ServiceInitiator serviceInitiator) { + thisLock.lock(); + try { + // todo : add check/error for unexpected initiator types? + return ( (StandardServiceInitiator) serviceInitiator ).initiateService( configurationValues, this ); + } finally { + thisLock.unlock(); } } @Override - public synchronized void destroy() { - super.destroy(); - this.configurationValues = null; + public void configureService(ServiceBinding serviceBinding) { + thisLock.lock(); + try { + if ( serviceBinding.getService() instanceof Configurable ) { + ( (Configurable) serviceBinding.getService() ).configure( configurationValues ); + } + } finally { + thisLock.unlock(); + } + } + + @Override + public void destroy() { + thisLock.lock(); + try { + super.destroy(); + this.configurationValues = null; + } finally { + thisLock.unlock(); + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/AbstractRegionFactory.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/AbstractRegionFactory.java index b8ecfaa62f..b73148c6eb 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/AbstractRegionFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/AbstractRegionFactory.java @@ -9,6 +9,8 @@ package org.hibernate.cache.spi; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.hibernate.boot.spi.SessionFactoryOptions; import org.hibernate.cache.CacheException; @@ -46,6 +48,7 @@ public abstract class AbstractRegionFactory implements RegionFactory { private SessionFactoryOptions options; + protected final Lock thisLock = new ReentrantLock(); protected boolean isStarted() { if ( started.get() ) { @@ -83,7 +86,8 @@ public abstract class AbstractRegionFactory implements RegionFactory { @Override public final void start(SessionFactoryOptions settings, Map configValues) throws CacheException { if ( started.compareAndSet( false, true ) ) { - synchronized (this) { + thisLock.lock(); + try { this.options = settings; try { prepareForUse( settings, configValues ); @@ -94,6 +98,8 @@ public abstract class AbstractRegionFactory implements RegionFactory { started.set( false ); startingException = e; } + } finally { + thisLock.unlock(); } } else { @@ -106,7 +112,8 @@ public abstract class AbstractRegionFactory implements RegionFactory { @Override public final void stop() { if ( started.compareAndSet( true, false ) ) { - synchronized ( this ) { + thisLock.lock(); + try { try { releaseFromUse(); } @@ -114,6 +121,8 @@ public abstract class AbstractRegionFactory implements RegionFactory { options = null; startingException = null; } + } finally { + thisLock.unlock(); } } else { 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 aaca3c5ac1..e7b8a436d9 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 @@ -14,6 +14,8 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import org.hibernate.boot.registry.BootstrapServiceRegistry; @@ -72,6 +74,9 @@ public abstract class AbstractServiceRegistryImpl private final AtomicBoolean active = new AtomicBoolean( true ); + protected final Lock thisLock = new ReentrantLock(); + private final Lock serviceBindingListLock = new ReentrantLock(); + protected AbstractServiceRegistryImpl(@Nullable ServiceRegistryImplementor parent) { this( parent, true ); } @@ -199,7 +204,8 @@ public abstract class AbstractServiceRegistryImpl } //Any service initialization needs synchronization - synchronized ( this ) { + thisLock.lock(); + try { // Check again after having acquired the lock: service = serviceRole.cast( initializedServiceByRole.get( serviceRole ) ); if ( service != null ) { @@ -219,13 +225,18 @@ public abstract class AbstractServiceRegistryImpl initializedServiceByRole.put( serviceRole, service ); } return service; + } finally { + thisLock.unlock(); } } protected void registerService(ServiceBinding serviceBinding, R service) { serviceBinding.setService( service ); - synchronized ( serviceBindingList ) { + serviceBindingListLock.lock(); + try { serviceBindingList.add( serviceBinding ); + } finally { + serviceBindingListLock.unlock(); } } @@ -351,78 +362,101 @@ public abstract class AbstractServiceRegistryImpl } @Override - 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> serviceBindingsIterator = serviceBindingList.listIterator( - serviceBindingList.size() - ); - while ( serviceBindingsIterator.hasPrevious() ) { - final ServiceBinding serviceBinding = serviceBindingsIterator.previous(); - serviceBinding.getLifecycleOwner().stopService( serviceBinding ); + public void destroy() { + thisLock.lock(); + try { + 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(); + serviceBindingListLock.lock(); + try { + ListIterator> serviceBindingsIterator = serviceBindingList.listIterator( + serviceBindingList.size() + ); + while ( serviceBindingsIterator.hasPrevious() ) { + final ServiceBinding serviceBinding = serviceBindingsIterator.previous(); + serviceBinding.getLifecycleOwner().stopService( serviceBinding ); + } + serviceBindingList.clear(); + } finally { + serviceBindingListLock.unlock(); } - serviceBindingList.clear(); + serviceBindingMap.clear(); } - serviceBindingMap.clear(); - } - finally { - if ( parent != null ) { - parent.deRegisterChild( this ); + finally { + if ( parent != null ) { + parent.deRegisterChild( this ); + } } } + } finally { + thisLock.unlock(); } } @Override - public synchronized void stopService(ServiceBinding binding) { - final Service service = binding.getService(); - if ( service instanceof Stoppable ) { - try { - ( (Stoppable) service ).stop(); - } - catch ( Exception e ) { - log.unableToStopService( service.getClass(), e ); + public void stopService(ServiceBinding binding) { + thisLock.lock(); + try { + final Service service = binding.getService(); + if ( service instanceof Stoppable ) { + try { + ( (Stoppable) service ).stop(); + } + catch ( Exception e ) { + log.unableToStopService( service.getClass(), e ); + } } + } finally { + thisLock.unlock(); } } @Override - public synchronized void registerChild(ServiceRegistryImplementor child) { - if ( childRegistries == null ) { - childRegistries = new HashSet<>(); - } - if ( !childRegistries.add( child ) ) { - log.warnf( - "Child ServiceRegistry [%s] was already registered; this will end badly later...", - child - ); - } - } - - @Override - public synchronized void deRegisterChild(ServiceRegistryImplementor child) { - if ( childRegistries == null ) { - throw new IllegalStateException( "No child ServiceRegistry registrations found" ); - } - childRegistries.remove( child ); - if ( childRegistries.isEmpty() ) { - if ( autoCloseRegistry ) { - log.debug( - "Implicitly destroying ServiceRegistry on de-registration " + - "of all child ServiceRegistries" - ); - destroy(); + public void registerChild(ServiceRegistryImplementor child) { + thisLock.lock(); + try { + if ( childRegistries == null ) { + childRegistries = new HashSet<>(); } - else { - log.debug( - "Skipping implicitly destroying ServiceRegistry on de-registration " + - "of all child ServiceRegistries" + if ( !childRegistries.add( child ) ) { + log.warnf( + "Child ServiceRegistry [%s] was already registered; this will end badly later...", + child ); } + } finally { + thisLock.unlock(); + } + } + + @Override + public void deRegisterChild(ServiceRegistryImplementor child) { + thisLock.lock(); + try { + if ( childRegistries == null ) { + throw new IllegalStateException( "No child ServiceRegistry registrations found" ); + } + childRegistries.remove( child ); + if ( childRegistries.isEmpty() ) { + if ( autoCloseRegistry ) { + log.debug( + "Implicitly destroying ServiceRegistry on de-registration " + + "of all child ServiceRegistries" + ); + destroy(); + } + else { + log.debug( + "Skipping implicitly destroying ServiceRegistry on de-registration " + + "of all child ServiceRegistries" + ); + } + } + } finally { + thisLock.unlock(); } } @@ -430,19 +464,24 @@ public abstract class AbstractServiceRegistryImpl * Not intended for general use. We need the ability to stop and "reactivate" a registry to allow * experimentation with technologies such as GraalVM, Quarkus and Cri-O. */ - public synchronized void resetParent(@Nullable BootstrapServiceRegistry newParent) { - if ( this.parent != null ) { - this.parent.deRegisterChild( this ); - } - if ( newParent != null ) { - if ( !(newParent instanceof ServiceRegistryImplementor) ) { - throw new IllegalArgumentException( "ServiceRegistry parent needs to implement ServiceRegistryImplementor" ); + public void resetParent(@Nullable BootstrapServiceRegistry newParent) { + thisLock.lock(); + try { + if ( this.parent != null ) { + this.parent.deRegisterChild( this ); } - this.parent = (ServiceRegistryImplementor) newParent; - this.parent.registerChild( this ); - } - else { - this.parent = null; + if ( newParent != null ) { + if ( !(newParent instanceof ServiceRegistryImplementor) ) { + throw new IllegalArgumentException( "ServiceRegistry parent needs to implement ServiceRegistryImplementor" ); + } + this.parent = (ServiceRegistryImplementor) newParent; + this.parent.registerChild( this ); + } + else { + this.parent = null; + } + } finally { + thisLock.unlock(); } } @@ -477,9 +516,14 @@ public abstract class AbstractServiceRegistryImpl * Not intended for general use. We need the ability to stop and "reactivate" a registry to allow * experimentation with technologies such as GraalVM, Quarkus and Cri-O. */ - public synchronized void reactivate() { - if ( !active.compareAndSet( false, true ) ) { - throw new IllegalStateException( "Was not inactive, could not reactivate" ); + public void reactivate() { + thisLock.lock(); + try { + if ( !active.compareAndSet( false, true ) ) { + throw new IllegalStateException( "Was not inactive, could not reactivate" ); + } + } finally { + thisLock.unlock(); } }