[GitHub] [tomcat] martin-g commented on a change in pull request #379: Fix BZ 64080 - graceful close
martin-g commented on a change in pull request #379:
URL: https://github.com/apache/tomcat/pull/379#discussion_r530317113
##
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##
@@ -1302,6 +1320,16 @@ protected long countDownConnection() {
*/
public final void closeServerSocketGraceful() {
if (bindState == BindState.BOUND_ON_START) {
+// Stop accepting new connections
+acceptor.stop(-1);
+// Release locks that may be preventing the acceptor from stopping
+releaseConnectionLatch();
+unlockAccept();
+// Signal to any multiplexed protocols (HTTP/2) that they may wish
+// to stop accepting new streams
+getHandler().pause();
+// Update the bindSatte. This has the side-effect of disabling
Review comment:
s/bindSatte/bindState/
##
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##
@@ -1312,6 +1340,30 @@ public final void closeServerSocketGraceful() {
}
+/**
+ * Wait for the client connections to the server to close gracefully. The
+ * method will return when all of the client connections have closed or the
+ * method has been waiting for {@code waitTimeMillis}.
+ *
+ * @param waitMillisThe maximum time to wait in milliseconds for the
+ * client connections to close.
+ *
+ * @return The wait time, if any remaining when the method returned
+ */
+public final long closeConnectionsAwait(long waitMillis) {
Review comment:
Maybe the method name should be `awaitConnectionsClose` ?
Now it sounds like it closes and waits, but actually it just waits.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #379: Fix BZ 64080 - graceful close
markt-asf commented on a change in pull request #379:
URL: https://github.com/apache/tomcat/pull/379#discussion_r530331336
##
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##
@@ -1302,6 +1320,16 @@ protected long countDownConnection() {
*/
public final void closeServerSocketGraceful() {
if (bindState == BindState.BOUND_ON_START) {
+// Stop accepting new connections
+acceptor.stop(-1);
+// Release locks that may be preventing the acceptor from stopping
+releaseConnectionLatch();
+unlockAccept();
+// Signal to any multiplexed protocols (HTTP/2) that they may wish
+// to stop accepting new streams
+getHandler().pause();
+// Update the bindSatte. This has the side-effect of disabling
Review comment:
Fixed. Tx for the review.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #379: Fix BZ 64080 - graceful close
markt-asf commented on a change in pull request #379:
URL: https://github.com/apache/tomcat/pull/379#discussion_r530331565
##
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##
@@ -1312,6 +1340,30 @@ public final void closeServerSocketGraceful() {
}
+/**
+ * Wait for the client connections to the server to close gracefully. The
+ * method will return when all of the client connections have closed or the
+ * method has been waiting for {@code waitTimeMillis}.
+ *
+ * @param waitMillisThe maximum time to wait in milliseconds for the
+ * client connections to close.
+ *
+ * @return The wait time, if any remaining when the method returned
+ */
+public final long closeConnectionsAwait(long waitMillis) {
Review comment:
Good suggestion. Fixed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64872] Inefficient enum resolution in JSPs
https://bz.apache.org/bugzilla/show_bug.cgi?id=64872 --- Comment #22 from Mark Thomas --- Updated branch: https://github.com/markt-asf/tomcat/tree/bz-64872 The tests for the optimised ELInterpreter show about 10% improvement. The tests for the custom StringInterpreter (that handles attr="EnumConstant") show an order of magnitude improvement. If this looks good to you, the next step will be a discussion on the dev@ list to figure out which bits ship as part of standard Tomcat. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #379: Fix BZ 64080 - graceful close
rmaucher commented on pull request #379: URL: https://github.com/apache/tomcat/pull/379#issuecomment-733693070 Looks ok, at least I can't see right now why it would be bad. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated: Remove reused reference to NioChannel
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 2883d5c Remove reused reference to NioChannel
2883d5c is described below
commit 2883d5c6403bb17ea8c6112e5ed72170a38ef84b
Author: remm
AuthorDate: Wed Nov 25 13:56:52 2020 +0100
Remove reused reference to NioChannel
---
java/org/apache/tomcat/util/net/NioEndpoint.java | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index ab2766b..0f71519 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1134,8 +1134,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
private int fillReadBuffer(boolean block, ByteBuffer buffer) throws
IOException {
int n = 0;
-NioChannel socket = getSocket();
-if (socket == NioChannel.CLOSED_NIO_CHANNEL) {
+if (getSocket() == NioChannel.CLOSED_NIO_CHANNEL) {
throw new ClosedChannelException();
}
if (block) {
@@ -1152,7 +1151,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
throw new SocketTimeoutException();
}
}
-n = socket.read(buffer);
+n = getSocket().read(buffer);
if (n == -1) {
throw new EOFException();
} else if (n == 0) {
@@ -1176,7 +1175,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
}
} while (n == 0); // TLS needs to loop as reading zero
application bytes is possible
} else {
-n = socket.read(buffer);
+n = getSocket().read(buffer);
if (n == -1) {
throw new EOFException();
}
@@ -1188,8 +1187,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
@Override
protected void doWrite(boolean block, ByteBuffer buffer) throws
IOException {
int n = 0;
-NioChannel socket = getSocket();
-if (socket == NioChannel.CLOSED_NIO_CHANNEL) {
+if (getSocket() == NioChannel.CLOSED_NIO_CHANNEL) {
throw new ClosedChannelException();
}
if (block) {
@@ -1206,7 +1204,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
throw new SocketTimeoutException();
}
}
-n = socket.write(buffer);
+n = getSocket().write(buffer);
if (n == -1) {
throw new EOFException();
} else if (n == 0) {
@@ -1239,7 +1237,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
// write registration.
} else {
do {
-n = socket.write(buffer);
+n = getSocket().write(buffer);
if (n == -1) {
throw new EOFException();
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64872] Inefficient enum resolution in JSPs
https://bz.apache.org/bugzilla/show_bug.cgi?id=64872 --- Comment #23 from John Engebretson --- My constant reaction is "I'm glad you're doing this and not me." Your expertise is creating a far better implementation than I did! Thank you for your hard work. The code looks solid; the only thing that caught my eye was the non-static logger. That's typically bad practice for production code. Are you sure it's necessary? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 47750] ISAPI: Loss of worker settings when changing via jkstatus
https://bz.apache.org/bugzilla/show_bug.cgi?id=47750 Locate Review changed: What|Removed |Added URL||https://www.locatereview.co ||m/2020/11/shoes-for-walking ||-on-concrete.html --- Comment #9 from Locate Review --- config really looks resonable.(In reply to robert.mawer from comment #2) > Created attachment 24198 [details] > workers.properties file -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix BZ 64080 - graceful close
This is an automated email from the ASF dual-hosted git repository.
markt 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 b1133ad Fix BZ 64080 - graceful close
b1133ad is described below
commit b1133ada73a48b738793de1827ae4ca9735babb3
Author: Mark Thomas
AuthorDate: Thu Nov 19 11:06:55 2020 +
Fix BZ 64080 - graceful close
https://bz.apache.org/bugzilla/show_bug.cgi?id=64080
---
java/org/apache/catalina/core/StandardService.java | 35 --
java/org/apache/coyote/AbstractProtocol.java | 8
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/ProtocolHandler.java| 13 +
.../apache/tomcat/util/net/AbstractEndpoint.java | 56 +-
webapps/docs/changelog.xml | 7 +++
webapps/docs/config/service.xml| 10
7 files changed, 123 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardService.java
b/java/org/apache/catalina/core/StandardService.java
index 05965ca..5597cb0 100644
--- a/java/org/apache/catalina/core/StandardService.java
+++ b/java/org/apache/catalina/core/StandardService.java
@@ -105,8 +105,21 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
protected final MapperListener mapperListener = new MapperListener(this);
+private long gracefulStopAwaitMillis = 0;
+
+
// - Properties
+public long getGracefulStopAwaitMillis() {
+return gracefulStopAwaitMillis;
+}
+
+
+public void setGracefulStopAwaitMillis(long gracefulStopAwaitMillis) {
+this.gracefulStopAwaitMillis = gracefulStopAwaitMillis;
+}
+
+
@Override
public Mapper getMapper() {
return mapper;
@@ -453,21 +466,33 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@Override
protected void stopInternal() throws LifecycleException {
-// Pause connectors first
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.pause();
-// Close server socket if bound on start
-// Note: test is in AbstractEndpoint
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);
+}
+}
+
+// Pause the connectors
+for (Connector connector: connectors) {
+connector.pause();
+}
}
if(log.isInfoEnabled())
log.info(sm.getString("standardService.stop.name", this.name));
setState(LifecycleState.STOPPING);
-// Stop our defined Container second
+// Stop our defined Container once the Connectors are all paused
if (engine != null) {
synchronized (engine) {
engine.stop();
diff --git a/java/org/apache/coyote/AbstractProtocol.java
b/java/org/apache/coyote/AbstractProtocol.java
index c226b72..089a07c 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -734,6 +734,14 @@ public abstract class AbstractProtocol implements
ProtocolHandler,
}
+@Override
+public long awaitConnectionsClose(long waitMillis) {
+getLog().info(sm.getString("abstractProtocol.closeConnectionsAwait",
+Long.valueOf(waitMillis), getName()));
+return endpoint.awaitConnectionsClose(waitMillis);
+}
+
+
private void logPortOffset() {
if (getPort() != getPortWithOffset()) {
getLog().info(sm.getString("abstractProtocolHandler.portOffset",
getName(),
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index 3009a36..595cfb2 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -34,6 +34,7 @@ abstractProcessor.pushrequest.notsupported=Server push
requests are not supporte
abstractProcessor.setErrorState=Error state [{0}] reported while processing
request
abstractProcessor.socket.ssl=Exception getting SSL attributes
+abstractProtocol.closeConnectionsAwait=Waiting [{0}] milliseconds for existing
connections to [{1}] to complete and close.
abstractProtocol.mbeanDeregistrationFailed=Failed to de
[tomcat] branch master updated: Fix BZ 64080 - graceful close
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new a1cbdab Fix BZ 64080 - graceful close
a1cbdab is described below
commit a1cbdab44b243515a5e8a9fec3c6fa1ac9ba254a
Author: Mark Thomas
AuthorDate: Thu Nov 19 11:06:55 2020 +
Fix BZ 64080 - graceful close
https://bz.apache.org/bugzilla/show_bug.cgi?id=64080
---
java/org/apache/catalina/core/StandardService.java | 35 --
java/org/apache/coyote/AbstractProtocol.java | 8
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/ProtocolHandler.java| 13 +
.../apache/tomcat/util/net/AbstractEndpoint.java | 56 +-
webapps/docs/changelog.xml | 7 +++
webapps/docs/config/service.xml| 10
7 files changed, 123 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardService.java
b/java/org/apache/catalina/core/StandardService.java
index 05965ca..5597cb0 100644
--- a/java/org/apache/catalina/core/StandardService.java
+++ b/java/org/apache/catalina/core/StandardService.java
@@ -105,8 +105,21 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
protected final MapperListener mapperListener = new MapperListener(this);
+private long gracefulStopAwaitMillis = 0;
+
+
// - Properties
+public long getGracefulStopAwaitMillis() {
+return gracefulStopAwaitMillis;
+}
+
+
+public void setGracefulStopAwaitMillis(long gracefulStopAwaitMillis) {
+this.gracefulStopAwaitMillis = gracefulStopAwaitMillis;
+}
+
+
@Override
public Mapper getMapper() {
return mapper;
@@ -453,21 +466,33 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@Override
protected void stopInternal() throws LifecycleException {
-// Pause connectors first
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.pause();
-// Close server socket if bound on start
-// Note: test is in AbstractEndpoint
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);
+}
+}
+
+// Pause the connectors
+for (Connector connector: connectors) {
+connector.pause();
+}
}
if(log.isInfoEnabled())
log.info(sm.getString("standardService.stop.name", this.name));
setState(LifecycleState.STOPPING);
-// Stop our defined Container second
+// Stop our defined Container once the Connectors are all paused
if (engine != null) {
synchronized (engine) {
engine.stop();
diff --git a/java/org/apache/coyote/AbstractProtocol.java
b/java/org/apache/coyote/AbstractProtocol.java
index e6fbaed..921d78b 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -711,6 +711,14 @@ public abstract class AbstractProtocol implements
ProtocolHandler,
}
+@Override
+public long awaitConnectionsClose(long waitMillis) {
+getLog().info(sm.getString("abstractProtocol.closeConnectionsAwait",
+Long.valueOf(waitMillis), getName()));
+return endpoint.awaitConnectionsClose(waitMillis);
+}
+
+
private void logPortOffset() {
if (getPort() != getPortWithOffset()) {
getLog().info(sm.getString("abstractProtocolHandler.portOffset",
getName(),
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index 3009a36..595cfb2 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -34,6 +34,7 @@ abstractProcessor.pushrequest.notsupported=Server push
requests are not supporte
abstractProcessor.setErrorState=Error state [{0}] reported while processing
request
abstractProcessor.socket.ssl=Exception getting SSL attributes
+abstractProtocol.closeConnectionsAwait=Waiting [{0}] milliseconds for existing
connections to [{1}] to complete and close.
abstractProtocol.mbeanDeregistrationFailed=Failed to
[Bug 64080] Graceful shutdown does not occur for connected clients that have not yet submitted their request payload
https://bz.apache.org/bugzilla/show_bug.cgi?id=64080 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #14 from Mark Thomas --- Fixed in: - 10.0.x for 10.0.0-M11 onwards - 9.0.x for 9.0.41 onwards Back-port to 8.5.x and earlier isn't practical due to the extent of the refactoring between 8.5.x and 9.0.x particularly around the Acceptor. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64872] Inefficient enum resolution in JSPs
https://bz.apache.org/bugzilla/show_bug.cgi?id=64872 --- Comment #24 from Mark Thomas --- There can be issues around loggers and class loading with Jasper. Tomcat has a custom LogManager that ID's logger by a combination of name and ThreadContextClassLoader (rather than just name). Depending on when the class is loaded you can see issues. These classes were affected but there are ways around that. I'll take a look. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
buildbot success in on tomcat-9-trunk
The Buildbot has detected a restored build on builder tomcat-9-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-9-trunk/builds/555 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: asf946_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-9-commit' triggered this build Build Source Stamp: [branch 9.0.x] b1133ada73a48b738793de1827ae4ca9735babb3 Blamelist: Mark Thomas Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64858] Allow to deploy a WAR whose dependencies are on a Maven repository
https://bz.apache.org/bugzilla/show_bug.cgi?id=64858 --- Comment #8 from Gael Lalire --- (In reply to Mark Thomas from comment #7) > Which code are you referring to here? The StandardJarScanner obtains URLs > for the JARs via the WebResources implementation. StandardJarScanner.process is expecting either "jar:" or "file:" URL, so even if WebResource is allowing any URL we are forced to use one of these. The good think is that JarScannerCallback is now using a org.apache.tomcat.Jar so if I can overload StandardJarScanner.process it will be good but : - UrlJar is opening a connection and cast in JarURLConnection - FragmentJarScannerCallback.extractJarFileName is expecting a jar URL. - TagLibraryInfoImpl. is opening a connection instead of using an inexistent Jar.getLastModified() which could call WebResource.getLastModified - StandardRoot.BaseLocation. is expecting either jar or file. With jar it is expected a file inside. Should also use JarFactory.newInstance instead. - maybe other ... However it is not an issue for the moment I can use file and jar URL and still load from Maven repository. (In reply to Mark Thomas from comment #7) > My preference continues to be doing this via WebResources and making changes > > there as necessary to permit extensions like this. OK, I updated my PR putting only what I need. I changed only 5 tomcat files with minor changes (https://github.com/apache/tomcat/pull/375/files). I understand that the TomcatController I proposed is too invasive. Can you guide me on how to transform it so it becomes acceptable ? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64872] Inefficient enum resolution in JSPs
https://bz.apache.org/bugzilla/show_bug.cgi?id=64872 --- Comment #25 from Christopher Schultz --- I noticed that some of the boxed/primitive handling (e.g. boolean, char) are handled in separate branches and others (e.g. long, int, short) are handled together in a single branch (with sub-branches using type.isPrimitive). Is there any reason to use the two separate styles or did you just change your mind halfway through the implementation? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
