This is an automated email from the ASF dual-hosted git repository.
schultz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new d8c7d269bc Improve the locking stratgey for StandardServer.services
d8c7d269bc is described below
commit d8c7d269bcaa5db62dcf3ff2768c903634c6fa8f
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 | 6 ++
2 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 68dc96ce91..5d102c697f 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -31,6 +31,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;
@@ -135,8 +137,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.
@@ -501,7 +509,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;
@@ -517,8 +527,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() {
@@ -685,13 +696,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;
}
@@ -701,8 +717,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
*/
@Override
public Service[] findServices() {
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
return services.clone();
+ } finally {
+ servicesReadLock.unlock();
}
}
@@ -710,12 +730,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();
}
}
@@ -728,7 +752,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]) {
@@ -755,8 +781,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
// Report this property change to interested listeners
support.firePropertyChange("service", service, null);
+ } finally {
+ servicesWriteLock.unlock();
}
-
}
@@ -912,10 +939,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) {
@@ -965,10 +996,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) {
@@ -1008,20 +1043,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 86243c6a61..b2be982712 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,12 @@
Add the Jakarta EE 11 XML schemas and update Tomcat and included web
applications to use them. (markt)
</add>
+ <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">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
