Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression on graceful shutdown default in Jetty 10 #6329

Closed
ofrias opened this issue May 27, 2021 · 8 comments
Closed

Regression on graceful shutdown default in Jetty 10 #6329

ofrias opened this issue May 27, 2021 · 8 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@ofrias
Copy link

ofrias commented May 27, 2021

The method Servlet.destroy() is not being called when Jetty is stopped when using the default configuration.

It is called if this line is added to the configuration:

jetty.server.stopAtShutdown=true

But it seems to me that the default behavior should be the opposite, because calling Servlet.destroy() is what one expects from the servlet container, and not calling it looks like an optional optimization.

@joakime
Copy link
Contributor

joakime commented May 27, 2021

The jetty.server.stopAtShutdown configuration option causes the Server.setStopAtShutdown(boolean) method to be called in the etc/jetty.xml configuration.

This essentially controls how the server behaves with a SIGTERM (or Ctrl+C command).

This configuration does not impact normal graceful shutdown done with the ShutdownMonitor and the stop/shutdown commands.

The defaults in Jetty are ...

  • Normal graceful shutdown is done via ShutdownMonitor and stop/shutdown commands (like what is seen in start.jar --stop or the jetty.sh stop scenarios).
  • Harsh stop with SIGTERM is fast and not graceful.

@ofrias
Copy link
Author

ofrias commented May 27, 2021

We use jetty.sh stop and Jetty does not call Servlet.destroy() if jetty.server.stopAtShutdown is not set to true

@joakime
Copy link
Contributor

joakime commented May 27, 2021

Since you didn't declare your specific version of Jetty or OS, or where you got your copy of Jetty (eg: from a Linux distro packaging), this is the generic response ...

If your usage of jetty.sh stop doesn't behave, it's likely that you haven't declared STOP.PORT and/or STOP.KEY.
Have you declared them in your /etc/default/jetty ?

Once your copy of Jetty is running, the port your defined in STOP.PORT will be listening on localhost only. (You can verify this with netstat)

You can verify the STOP.PORT and STOP.KEY behavior with either start.jar --stop or netcat.

Example: if you have STOP.PORT=7888 and STOP.KEY=freebeer then this should cause a graceful stop.

start.jar version

$ cd /path/to/my-jetty-base
$ java -DSTOP.PORT=7888 -DSTOP.KEY=freebeer -jar /path/to/my-jetty-home/start.jar --stop

Note: the STOP.PORT and STOP.KEY values are declared already when using jetty.sh as it pulls it in from /etc/default/jetty, but when doing this from the command line you'll have to declare them manually.

Netcat version

$ echo -en 'freebeer\nstopexit\n' | nc 127.0.0.1 7888

Now as to why your usage of jetty.sh stop doesn't behave as expected, it could be because you don't have STOP.PORT and/or STOP.KEY defined, then the start-stop-daemon can fallback to use kill -HUP <pid> (in essence)

This behavior is resistant to documentation as it's highly dependent on your version of Jetty, the version of start-stop-daemon you use, the OS you use, and the origin of your jetty.sh (such as getting it from a linux distro packaging vs from the official tarballs directly).

@ofrias
Copy link
Author

ofrias commented May 27, 2021

We are using the jetty.sh shipped in Jetty 10.0.3 tarball. We have not configured STOP.PORT and STOP.KEY in our /etc/default/jetty, but they were also not configured for Jetty 9.4 and it worked as expected with kill signals. In our case we have fixed this by setting the jetty.server.stopAtShutdown=true flag, but in my opinion the default setup should stop gracefully when using jetty.sh without any extra configuration (as it was with Jetty 9.4 and before).

@joakime
Copy link
Contributor

joakime commented May 27, 2021

Identifying the difference as compared between 9.4.x and 10.0.x is useful! Thank you for that.

Looking around ...

[joakim@hyperion distros]$ diff -u jetty-home-9.4.41.v20210516/bin/jetty.sh jetty-home-10.0.3/bin/jetty.sh
[joakim@hyperion distros]$

Well, the jetty.sh is identical. What about the jetty.xml ...

[joakim@hyperion distros]$ diff -u jetty-home-9.4.41.v20210516/etc/jetty.xml jetty-home-10.0.3/etc/jetty.xml
--- jetty-home-9.4.41.v20210516/etc/jetty.xml	2021-05-16 16:32:14.000000000 -0500
+++ jetty-home-10.0.3/etc/jetty.xml	2021-05-20 15:06:44.000000000 -0500
@@ -1,4 +1,5 @@
-<?xml version="1.0"?><!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 
 <!-- =============================================================== -->
 <!-- Documentation of this file format can be found at:              -->
@@ -58,38 +59,36 @@
     <!-- for all configuration that may be set here.                 -->
     <!-- =========================================================== -->
     <New id="httpConfig" class="org.eclipse.jetty.server.HttpConfiguration">
-      <Set name="secureScheme"><Property name="jetty.httpConfig.secureScheme" default="https" /></Set>
-      <Set name="securePort"><Property name="jetty.httpConfig.securePort" deprecated="jetty.secure.port" default="8443" /></Set>
-      <Set name="outputBufferSize"><Property name="jetty.httpConfig.outputBufferSize" deprecated="jetty.output.buffer.size" default="32768" /></Set>
-      <Set name="outputAggregationSize"><Property name="jetty.httpConfig.outputAggregationSize" deprecated="jetty.output.aggregation.size" default="8192" /></Set>
-      <Set name="requestHeaderSize"><Property name="jetty.httpConfig.requestHeaderSize" deprecated="jetty.request.header.size" default="8192" /></Set>
-      <Set name="responseHeaderSize"><Property name="jetty.httpConfig.responseHeaderSize" deprecated="jetty.response.header.size" default="8192" /></Set>
-      <Set name="sendServerVersion"><Property name="jetty.httpConfig.sendServerVersion" deprecated="jetty.send.server.version" default="true" /></Set>
-      <Set name="sendDateHeader"><Property name="jetty.httpConfig.sendDateHeader" deprecated="jetty.send.date.header" default="false" /></Set>
-      <Set name="headerCacheSize"><Property name="jetty.httpConfig.headerCacheSize" default="1024" /></Set>
-      <Set name="delayDispatchUntilContent"><Property name="jetty.httpConfig.delayDispatchUntilContent" deprecated="jetty.delayDispatchUntilContent" default="true"/></Set>
-      <Set name="maxErrorDispatches"><Property name="jetty.httpConfig.maxErrorDispatches" default="10"/></Set>
-      <Set name="blockingTimeout"><Property deprecated="jetty.httpConfig.blockingTimeout" name="jetty.httpConfig.blockingTimeout.DEPRECATED" default="-1"/></Set>
-      <Set name="persistentConnectionsEnabled"><Property name="jetty.httpConfig.persistentConnectionsEnabled" default="true"/></Set>
-      <Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" deprecated="jetty.httpConfig.cookieCompliance" default="RFC6265"/></Arg></Call></Set>
+      <Set name="secureScheme" property="jetty.httpConfig.secureScheme"/>
+      <Set name="securePort" property="jetty.httpConfig.securePort"/>
+      <Set name="outputBufferSize" property="jetty.httpConfig.outputBufferSize"/>
+      <Set name="outputAggregationSize" property="jetty.httpConfig.outputAggregationSize"/>
+      <Set name="requestHeaderSize" property="jetty.httpConfig.requestHeaderSize"/>
+      <Set name="responseHeaderSize" property="jetty.httpConfig.responseHeaderSize"/>
+      <Set name="sendServerVersion" property="jetty.httpConfig.sendServerVersion"/>
+      <Set name="sendDateHeader" property="jetty.httpConfig.sendDateHeader"/>
+      <Set name="headerCacheSize" property="jetty.httpConfig.headerCacheSize"/>
+      <Set name="delayDispatchUntilContent" property="jetty.httpConfig.delayDispatchUntilContent"/>
+      <Set name="maxErrorDispatches" property="jetty.httpConfig.maxErrorDispatches"/>
+      <Set name="persistentConnectionsEnabled" property="jetty.httpConfig.persistentConnectionsEnabled"/>
+      <Set name="httpCompliance"><Call class="org.eclipse.jetty.http.HttpCompliance" name="from"><Arg><Property name="jetty.httpConfig.compliance" deprecated="jetty.http.compliance" default="RFC7230"/></Arg></Call></Set>
+      <Set name="uriCompliance"><Call class="org.eclipse.jetty.http.UriCompliance" name="from"><Arg><Property name="jetty.httpConfig.uriCompliance" default="SAFE"/></Arg></Call></Set>
+      <Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" default="RFC6265"/></Arg></Call></Set>
       <Set name="responseCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.responseCookieCompliance" default="RFC6265"/></Arg></Call></Set>
-      <Set name="multiPartFormDataCompliance"><Call class="org.eclipse.jetty.server.MultiPartFormDataCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.multiPartFormDataCompliance" default="RFC7578"/></Arg></Call></Set>
       <Set name="relativeRedirectAllowed"><Property name="jetty.httpConfig.relativeRedirectAllowed" default="false"/></Set>
+      <Set name="useInputDirectByteBuffers" property="jetty.httpConfig.useInputDirectByteBuffers"></Set>
+      <Set name="useOutputDirectByteBuffers" property="jetty.httpConfig.useOutputDirectByteBuffers"></Set>
     </New>
 
     <!-- =========================================================== -->
     <!-- Set the default handler structure for the Server            -->
-    <!-- A handler collection is used to pass received requests to   -->
+    <!-- A handler list is used to pass received requests to         -->
     <!-- both the ContextHandlerCollection, which selects the next   -->
-    <!-- handler by context path and virtual host, and the           -->
-    <!-- DefaultHandler, which handles any requests not handled by   -->
-    <!-- the context handlers.                                       -->
-    <!-- Other handlers may be added to the "Handlers" collection,   -->
-    <!-- for example the jetty-requestlog.xml file adds the          -->
-    <!-- RequestLogHandler after the default handler                 -->
+    <!-- handler by context path and virtual host, and then only if  -->
+    <!-- the request is not handled is the DefaultHandler invoked    -->
     <!-- =========================================================== -->
     <Set name="handler">
-      <New id="Handlers" class="org.eclipse.jetty.server.handler.HandlerCollection">
+      <New id="Handlers" class="org.eclipse.jetty.server.handler.HandlerList">
         <Set name="handlers">
          <Array type="org.eclipse.jetty.server.Handler">
            <Item>
@@ -106,9 +105,9 @@
     <!-- =========================================================== -->
     <!-- extra server options                                        -->
     <!-- =========================================================== -->
-    <Set name="stopAtShutdown"><Property name="jetty.server.stopAtShutdown" default="true"/></Set>
+    <Set name="stopAtShutdown" property="jetty.server.stopAtShutdown"/>
     <Set name="stopTimeout"><Property name="jetty.server.stopTimeout" default="5000"/></Set>
-    <Set name="dumpAfterStart"><Property name="jetty.server.dumpAfterStart" deprecated="jetty.dump.start" default="false"/></Set>
-    <Set name="dumpBeforeStop"><Property name="jetty.server.dumpBeforeStop" deprecated="jetty.dump.stop" default="false"/></Set>
+    <Set name="dumpAfterStart" property="jetty.server.dumpAfterStart"/>
+    <Set name="dumpBeforeStop" property="jetty.server.dumpBeforeStop"/>
 
 </Configure>
[joakim@hyperion distros]$ 

Aha! the syntax change on XML is the culprit.

Specifically this ...

-    <Set name="stopAtShutdown"><Property name="jetty.server.stopAtShutdown" default="true"/></Set>
+    <Set name="stopAtShutdown" property="jetty.server.stopAtShutdown"/>

This change is a result of commit 343cf73
Which missed a few default attribute settings in the XMLs when converting to the new syntax.

That entire commit will need to be reviewed.

@joakime joakime added the Bug For general bugs on Jetty side label May 27, 2021
@joakime joakime changed the title Servlet.destroy() not called on Jetty 10 stop Regression on graceful shutdown default in Jetty 10 May 27, 2021
@gregw
Copy link
Contributor

gregw commented May 27, 2021

@joakime good catch. The new xml does not call the setter unless the property is set, so it is no good for setting a default.

We can either change the default in the code or revert those xml segments

@janbartel janbartel self-assigned this Jun 2, 2021
janbartel added a commit that referenced this issue Jun 2, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Jun 3, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Jun 8, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Jun 9, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Jun 10, 2021
Signed-off-by: Jan Bartel <janb@webtide.com>
@janbartel
Copy link
Contributor

I've raised an issue which more correctly reflects the fact that we've done a full review of the xml changes from commit 343cf73: #6392. So this issue will be fixed by the PR (#6348) for that issue.

janbartel added a commit that referenced this issue Jun 10, 2021
* Issue #6392 Review accidental xml config changes

Signed-off-by: Jan Bartel <janb@webtide.com>
@janbartel
Copy link
Contributor

Closed via PR #6348

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

4 participants