This is an automated email from the ASF dual-hosted git repository.
schultz pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 0b3ee30d14 Improve the locking stratgey for StandardServer.services
0b3ee30d14 is described below
commit 0b3ee30d148e64a2f4ec73de7d236a05e9f4a6d9
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 2b2ae007e2..9175da846b 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -35,6 +35,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;
@@ -140,8 +142,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.
@@ -506,7 +514,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;
@@ -522,8 +532,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() {
@@ -693,13 +704,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;
}
@@ -709,8 +725,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
*/
@Override
public Service[] findServices() {
- synchronized (servicesLock) {
+ servicesReadLock.lock();
+
+ try {
return services.clone();
+ } finally {
+ servicesReadLock.unlock();
}
}
@@ -718,12 +738,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();
}
}
@@ -736,7 +760,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]) {
@@ -763,8 +789,9 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
// Report this property change to interested listeners
support.firePropertyChange("service", service, null);
+ } finally {
+ servicesWriteLock.unlock();
}
-
}
@@ -920,10 +947,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) {
@@ -973,10 +1004,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) {
@@ -1041,20 +1076,28 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
}
}
// 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 b36849fec5..3140656f20 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -124,6 +124,12 @@
<code>NullPointerException</code>, if an attempt is made to use the
<code>AsyncContext</code> after it has been recycled. (markt)
</fix>
+ <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
