This is an automated email from the ASF dual-hosted git repository.
schultz pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 9c306cf92b Improve the locking stratgey for StandardServer.services
9c306cf92b is described below
commit 9c306cf92bae894e84dbf533d6c64b555869a711
Author: Christopher Schultz <ch...@christopherschultz.net>
AuthorDate: Tue Apr 2 11:00:14 2024 -0400
Improve the locking stratgey for StandardServer.services
Change a simple synchronized lock to a ReentrantReadWriteLock to allow
multiple readers to operate simultaneously. Based upon a suggestion by Markus
Wolfe.
---
java/org/apache/catalina/core/StandardServer.java | 67 +++++++++++++++++++----
webapps/docs/changelog.xml | 10 ++++
2 files changed, 65 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index e4ca5ec3d7..f77c5b38cd 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -32,6 +32,8 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import javax.management.InstanceNotFoundException;
import javax.management.MBeanException;
@@ -136,8 +138,14 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
* The set of Services associated with this Server.
*/
private Service[] services = new Service[0];
- private final Object servicesLock = new Object();
+ private final Lock servicesReadLock;
+ private final Lock servicesWriteLock;
+ {
+ ReentrantReadWriteLock servicesLock = new ReentrantReadWriteLock();
+ servicesReadLock = servicesLock.readLock();
+ servicesWriteLock = servicesLock.writeLock();
+ }
/**
* The shutdown command string we are looking for.
@@ -502,7 +510,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
service.setServer(this);
- synchronized (servicesLock) {
+ servicesWriteLock.lock();
+
+ try {
Service results[] = new Service[services.length + 1];
System.arraycopy(services, 0, results, 0, services.length);
results[services.length] = service;
@@ -518,8 +528,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
// Report this property change to interested listeners
support.firePropertyChange("service", null, service);
+ } finally {
+ servicesWriteLock.unlock();
}
-
}
public void stopAwait() {
@@ -689,13 +700,18 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
if (name == null) {
return null;
}
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
for (Service service : services) {
if (name.equals(service.getName())) {
return service;
}
}
+ } finally {
+ servicesReadLock.unlock();
}
+
return null;
}
@@ -705,8 +721,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
*/
@Override
public Service[] findServices() {
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
return services.clone();
+ } finally {
+ servicesReadLock.unlock();
}
}
@@ -714,12 +734,16 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
* @return the JMX service names.
*/
public ObjectName[] getServiceNames() {
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
ObjectName[] onames = new ObjectName[services.length];
for (int i = 0; i < services.length; i++) {
onames[i] = ((StandardService) services[i]).getObjectName();
}
return onames;
+ } finally {
+ servicesReadLock.unlock();
}
}
@@ -732,7 +756,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
@Override
public void removeService(Service service) {
- synchronized (servicesLock) {
+ servicesWriteLock.lock();
+
+ try {
int j = -1;
for (int i = 0; i < services.length; i++) {
if (service == services[i]) {
@@ -759,8 +785,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
// Report this property change to interested listeners
support.firePropertyChange("service", service, null);
+ } finally {
+ servicesWriteLock.unlock();
}
-
}
@@ -916,10 +943,14 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
globalNamingResources.start();
// Start our defined Services
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
for (Service service : services) {
service.start();
}
+ } finally {
+ servicesReadLock.unlock();
}
if (periodicEventDelay > 0) {
@@ -969,10 +1000,14 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
fireLifecycleEvent(CONFIGURE_STOP_EVENT, null);
// Stop our defined Services
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
for (Service service : services) {
service.stop();
}
+ } finally {
+ servicesReadLock.unlock();
}
synchronized (utilityExecutorLock) {
@@ -1012,20 +1047,28 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
globalNamingResources.init();
// Initialize our defined Services
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
for (Service service : services) {
service.init();
}
+ } finally {
+ servicesReadLock.unlock();
}
}
@Override
protected void destroyInternal() throws LifecycleException {
// Destroy our defined Services
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
for (Service service : services) {
service.destroy();
}
+ } finally {
+ servicesReadLock.unlock();
}
globalNamingResources.destroy();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a3839123e4..1675adf2b9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,16 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 10.1.21 (schultz)" rtext="in development">
+ <subsection name="Catalina">
+ <changelog>
+ <fix>
+ Change the thread-safety mechanism for protecting
StandardServer.services
+ from a simple synchronized lock to a ReentrantReadWriteLock to allow
+ multiple readers to operate simultaneously. Based upon a suggestion by
+ Markus Wolfe (schultz).
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Coyote">
<changelog>
<add>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
