This is an automated email from the ASF dual-hosted git repository.
remm 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 047daa0d0f Also add a lock for connectors
047daa0d0f is described below
commit 047daa0d0fc1ac7a388d1ee672d22501dae76e85
Author: remm <[email protected]>
AuthorDate: Wed Apr 3 16:49:11 2024 +0200
Also add a lock for connectors
---
java/org/apache/catalina/core/StandardService.java | 107 +++++++++++----------
webapps/docs/changelog.xml | 6 +-
2 files changed, 63 insertions(+), 50 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardService.java
b/java/org/apache/catalina/core/StandardService.java
index a194c3dd6e..dbb32e55ce 100644
--- a/java/org/apache/catalina/core/StandardService.java
+++ b/java/org/apache/catalina/core/StandardService.java
@@ -20,6 +20,9 @@ package org.apache.catalina.core;
import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
import java.util.ArrayList;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import javax.management.ObjectName;
@@ -76,7 +79,7 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
* The set of Connectors associated with this Service.
*/
protected Connector connectors[] = new Connector[0];
- private final Object connectorsLock = new Object();
+ private final ReadWriteLock connectorsLock = new ReentrantReadWriteLock();
/**
* The list of executors held by the service.
@@ -219,12 +222,16 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@Override
public void addConnector(Connector connector) {
- synchronized (connectorsLock) {
+ Lock writeLock = connectorsLock.writeLock();
+ writeLock.lock();
+ try {
connector.setService(this);
Connector results[] = new Connector[connectors.length + 1];
System.arraycopy(connectors, 0, results, 0, connectors.length);
results[connectors.length] = connector;
connectors = results;
+ } finally {
+ writeLock.unlock();
}
try {
@@ -241,12 +248,16 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
public ObjectName[] getConnectorNames() {
- synchronized (connectorsLock) {
+ Lock readLock = connectorsLock.readLock();
+ readLock.lock();
+ try {
ObjectName results[] = new ObjectName[connectors.length];
for (int i = 0; i < results.length; i++) {
results[i] = connectors[i].getObjectName();
}
return results;
+ } finally {
+ readLock.unlock();
}
}
@@ -266,9 +277,13 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
*/
@Override
public Connector[] findConnectors() {
- synchronized (connectorsLock) {
+ Lock readLock = connectorsLock.readLock();
+ readLock.lock();
+ try {
// shallow copy
return connectors.clone();
+ } finally {
+ readLock.unlock();
}
}
@@ -282,7 +297,9 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@Override
public void removeConnector(Connector connector) {
- synchronized (connectorsLock) {
+ Lock writeLock = connectorsLock.writeLock();
+ writeLock.lock();
+ try {
int j = -1;
for (int i = 0; i < connectors.length; i++) {
if (connector == connectors[i]) {
@@ -312,7 +329,10 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
// Report this property change to interested listeners
support.firePropertyChange("connector", connector, null);
+ } finally {
+ writeLock.unlock();
}
+
}
@@ -443,12 +463,10 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
mapperListener.start();
// Start our defined Connectors second
- synchronized (connectorsLock) {
- for (Connector connector : connectors) {
- // If it has already failed, don't try and start it
- if (connector.getState() != LifecycleState.FAILED) {
- connector.start();
- }
+ for (Connector connector : findConnectors()) {
+ // If it has already failed, don't try and start it
+ if (connector.getState() != LifecycleState.FAILED) {
+ connector.start();
}
}
}
@@ -463,28 +481,27 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@Override
protected void stopInternal() throws LifecycleException {
- synchronized (connectorsLock) {
- // Initiate a graceful stop for each connector
- // This will only work if the bindOnInit==false which is not the
- // default.
- for (Connector connector : connectors) {
- connector.getProtocolHandler().closeServerSocketGraceful();
- }
-
- // Wait for the graceful shutdown to complete
- long waitMillis = gracefulStopAwaitMillis;
- if (waitMillis > 0) {
- for (Connector connector : connectors) {
- waitMillis =
connector.getProtocolHandler().awaitConnectionsClose(waitMillis);
- }
- }
+ Connector[] connectors = findConnectors();
+ // Initiate a graceful stop for each connector
+ // This will only work if the bindOnInit==false which is not the
+ // default.
+ for (Connector connector : connectors) {
+ connector.getProtocolHandler().closeServerSocketGraceful();
+ }
- // Pause the connectors
+ // Wait for the graceful shutdown to complete
+ long waitMillis = gracefulStopAwaitMillis;
+ if (waitMillis > 0) {
for (Connector connector : connectors) {
- connector.pause();
+ waitMillis =
connector.getProtocolHandler().awaitConnectionsClose(waitMillis);
}
}
+ // Pause the connectors
+ for (Connector connector : connectors) {
+ connector.pause();
+ }
+
if (log.isInfoEnabled()) {
log.info(sm.getString("standardService.stop.name", this.name));
}
@@ -498,16 +515,14 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
}
// Now stop the connectors
- synchronized (connectorsLock) {
- for (Connector connector : connectors) {
- if (!LifecycleState.STARTED.equals(connector.getState())) {
- // Connectors only need stopping if they are currently
- // started. They may have failed to start or may have been
- // stopped (e.g. via a JMX call)
- continue;
- }
- connector.stop();
+ for (Connector connector : connectors) {
+ if (!LifecycleState.STARTED.equals(connector.getState())) {
+ // Connectors only need stopping if they are currently
+ // started. They may have failed to start or may have been
+ // stopped (e.g. via a JMX call)
+ continue;
}
+ connector.stop();
}
// If the Server failed to start, the mapperListener won't have been
@@ -516,10 +531,8 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
mapperListener.stop();
}
- synchronized (executors) {
- for (Executor executor : executors) {
- executor.stop();
- }
+ for (Executor executor : findExecutors()) {
+ executor.stop();
}
}
@@ -549,10 +562,8 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
mapperListener.init();
// Initialize our defined Connectors
- synchronized (connectorsLock) {
- for (Connector connector : connectors) {
- connector.init();
- }
+ for (Connector connector : findConnectors()) {
+ connector.init();
}
}
@@ -562,10 +573,8 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
mapperListener.destroy();
// Destroy our defined Connectors
- synchronized (connectorsLock) {
- for (Connector connector : connectors) {
- connector.destroy();
- }
+ for (Connector connector : findConnectors()) {
+ connector.destroy();
}
// Destroy any Executors
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 735429b85d..c7fbb6e342 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -128,7 +128,11 @@
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).
+ Markus Wolfe. (schultz)
+ </fix>
+ <fix>
+ Improve Service connectors access sync using a ReentrantReadWriteLock.
+ (remm)
</fix>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
