You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2021/10/29 09:20:32 UTC
[GitHub] [tomcat] michael-o opened a new pull request #456: Document conditions under which the AprLifecycleListener can be used …
michael-o opened a new pull request #456:
URL: https://github.com/apache/tomcat/pull/456
…to avoid JVM crashes
This basically document cases to avoid issues like
https://github.com/spring-projects/spring-boot/issues/28472
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #456:
URL: https://github.com/apache/tomcat/pull/456#discussion_r739135782
##########
File path: java/org/apache/catalina/core/AprLifecycleListener.java
##########
@@ -38,6 +38,13 @@
/**
* Implementation of <code>LifecycleListener</code> that will init and
* and destroy APR.
+ * <p>
+ * <strong>Note</strong>: This listener must only be used within a {@code Server}
+ * element to manage APR/OpenSSL init and destroy JVM-wide. If you are running
+ * Tomcat in an embedded fashion and have more than one Tomcat instance per JVM,
+ * this listener <em>must not</em> be added to the {@code Server} instance, but
Review comment:
I almost agree with your statement. What would you change in the documentation if the code remains the same? At least the current contract is fully defined.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-956045402
@ChristopherSchultz I have just generally copied the sentence (verbatim) from the website docs to listeners Javadoc.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o edited a comment on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-955175606
> Your test-case looks even more complicated than necessary: just initialize two of them then deinitialize them. No shutdown hook necessary, right?
This maybe true, but it replicates the behavior in Spring Boot. This is what I wanted to show you.
> Okay, so it's not crashing in the `AprLifecycleListener`'s shutdown, which is what it sounded like you were reporting.
I don't think I have actually written that. It is the APR protocol/endpoint.
> Certainly, anything can crash at any time after the APR global pools have been shut-down. We could put guards around those things. Something like this at the top of each of the calls which require APR:
>
> ```
> #DEFINE CHECK_APR_INITIALIZED(ENV) { \
> if(!tcn_global_pool) { \
> tcn_ThrowAPRException((ENV), APR_EINIT); \
> } \
> } \
>
> TCN_IMPLEMENT_CALL(jstring, Address, getnameinfo)(TCN_STDARGS,
> jlong sa, jint flags)
> {
> apr_sockaddr_t *s = J2P(sa, apr_sockaddr_t *);
> char *hostname;
>
> UNREFERENCED(o);
> CHECK_APR_INITIALIZED(e); /* <- This macro invocation is new */
> if (apr_getnameinfo(&hostname, s, (apr_int32_t)flags) == APR_SUCCESS)
> return AJP_TO_JSTRING(hostname);
> else
> return NULL;
> }
> ```
This sounds like a decent idea. It makes post mortem analysis much easier. It is work, but just manual one.
> It would be much cleaner to implement this at the Java level, but as your test-case demonstrates, it's always possible for APR to disappear during the execution of one of the native methods.
Agreed.
> I'm kind of curious as to exactly where in `Socket.accept()` this particular failure occurred. We use APR pools for things like string values, even to throw exceptions. So it's hard to avoid using APR anywhere. One could argue that APR pools aren't necessary for throwing exceptions, but re-writing tcnative at this point to remove APR-type things is unlikely. Perhaps incrementally. But Rémy's recent work with Project Panama looks like we might be able to dump tcnative in the reasonably-near future.
Unlikely since Tomat 9 and 10 will live very long and the minimum Java version is 8. Expect tomcat-native as of now to live for at least five more years. A future version with Panama will likely to have OpenSSL only, for now.
> Anyway, back to fixing this kind of thing:
>
> 1. I think it's worth mentioning the dangers of multiple AprLifecycleListeners in the Tomcat documentation. I don't think the PR as submitted goes far enough. I think it's even misleading at first (e.g. only use `AprLifecycleListener` on a `Server` element) because there are many ways to use the `AprLifecycleListener` in ways that can cause the JVM to crash. I think this documentation should be changed in the Javadoc but also in the documentation for the listener in general, here http://tomcat.apache.org/tomcat-10.0-doc/apr.html#APR_Lifecycle_Listener_Configuration and here http://tomcat.apache.org/tomcat-10.0-doc/config/listeners.html#APR_Lifecycle_Listener_-_org.apache.catalina.core.AprLifecycleListener. (Note that the second of these two already states that the listener should only be used on `<Server>` components.)
This is the first problem, the documentation you mention is intended for integrators and admins. Embedders read Javadoc only, as you have seen with Spring Boot. Wen should add at least minimal information to all of these listeners' Javadoc and maybe a link to the extended documentation. (our fault)
What would be your proposal for the documentation.
> 2. I think it's also reasonable to try to protect the JVM against this kind of failure. Anyone starting two Tomcats in a single JVM is going to have exactly the same problem. Yes, it's possible to have the "container" (e.g. Spring in this case) manage the whole, um, _lifecycle_ of the `AprLifecycleListener` but it's much more natural to use it "inside" Tomcat and allow Tomcat to manage that process.
Yes, natural as long as you abide and know the contract and requirements which partially did not happen in Spring Boot. I am inclined to add a WARNING log to all of these listeners when the `Lifecycle` is not of instance `Server`.
> I like the idea of reference-counting, especially because the number of times the `AprLifecycleListener` is initialized and de-initialized in a given JVM should be relatively low. It's a small amount of code to manage, provides a great amount of protection (JVM crashes should really never be tolerated), and the performance impact is irrelevant.
>
> Would you care to prepare a reference-counting patch for `AprLifecycleListener` either as a part of this PR, or as a separate one?
I would like to take care of the documentation shortage first and then discuss in a separate PR/thread reference counting approach.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz edited a comment on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz edited a comment on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954934999
> [...] so the second Tomcat now shuts down APR globally, releasing the global structures. The first Tomcat is now shutting down, its `AprConnector` tries to use a sub APR pool/object from a pool which has already been released.
There is no `AprConnector`. Where is the Java call which eventually goes down into native to cause this problem? As the AprLifecycleListener is stopping, it should not try to use any APR pool. Perhaps the other connector is still trying to actually use pool objects in some other way. If that's the case, the problem isn't with double-shutdown. It's with shutdown-while-trying-to-still-do-stuff.
> See
> * https://github.com/apache/tomcat-native/blob/c17d3e0e0a594604ff4f30065360aacc688cfb50/native/src/jnilib.c#L243-L262
Notably, even this native method prevents crashes due to double-shutdown. The predicate around the whole thing prevents it from being called twice.
So can we get a Java stack trace for where this is crashing? It should be simple to check to see if APR has been killed at some point and at least not crash the JVM.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-955018111
Your test-case looks even more complicated than necessary: just initialize two of them then deinitialize them. No shutdown hook necessary, right?
Okay, so it's not crashing in the `AprLifecycleListener`'s shutdown, which is what it sounded like you were reporting.
Certainly, anything can crash at any time after the APR global pools have been shut-down. We could put guards around those things. Something like this at the top of each of the calls which require APR:
```
#DEFINE CHECK_APR_INITIALIZED(ENV) { \
if(!tcn_global_pool) { \
tcn_ThrowAPRException((ENV), APR_EINIT); \
} \
} \
TCN_IMPLEMENT_CALL(jstring, Address, getnameinfo)(TCN_STDARGS,
jlong sa, jint flags)
{
apr_sockaddr_t *s = J2P(sa, apr_sockaddr_t *);
char *hostname;
UNREFERENCED(o);
CHECK_APR_INITIALIZED(e); /* <- This macro invocation is new */
if (apr_getnameinfo(&hostname, s, (apr_int32_t)flags) == APR_SUCCESS)
return AJP_TO_JSTRING(hostname);
else
return NULL;
}
```
It would be much cleaner to implement this at the Java level, but as your test-case demonstrates, it's always possible for APR to disappear during the execution of one of the native methods.
I'm kind of curious as to exactly where in `Socket.accept()` this particular failure occurred. We use APR pools for things like string values, even to throw exceptions. So it's hard to avoid using APR anywhere. One could argue that APR pools aren't necessary for throwing exceptions, but re-writing tcnative at this point to remove APR-type things is unlikely. Perhaps incrementally. But Rémy's recent work with Project Panama looks like we might be able to dump tcnative in the reasonably-near future.
Anyway, back to fixing this kind of thing:
1. I think it's worth mentioning the dangers of multiple AprLifecycleListeners in the Tomcat documentation. I don't think the PR as submitted goes far enough. I think it's even misleading at first (e.g. only use `AprLifecycleListener` on a `Server` element) because there are many ways to use the `AprLifecycleListener` in ways that can cause the JVM to crash. I think this documentation should be changed in the Javadoc but also in the documentation for the listener in general, here http://tomcat.apache.org/tomcat-10.0-doc/apr.html#APR_Lifecycle_Listener_Configuration and here http://tomcat.apache.org/tomcat-10.0-doc/config/listeners.html#APR_Lifecycle_Listener_-_org.apache.catalina.core.AprLifecycleListener. (Note that the second of these two already states that the listener should only be used on `<Server>` components.)
1. I think it's also reasonable to try to protect the JVM against this kind of failure. Anyone starting two Tomcats in a single JVM is going to have exactly the same problem. Yes, it's possible to have the "container" (e.g. Spring in this case) manage the whole, um, _lifecycle_ of the `AprLifecycleListener` but it's much more natural to use it "inside" Tomcat and allow Tomcat to manage that process.
I like the idea of reference-counting, especially because the number of times the `AprLifecycleListener` is initialized and de-initialized in a given JVM should be relatively low. It's a small amount of code to manage, provides a great amount of protection (JVM crashes should really never be tolerated), and the performance impact is irrelevant.
Would you care to prepare a reference-counting patch for `AprLifecycleListener` either as a part of this PR, or as a separate one?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o edited a comment on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954648720
>
>
> This feels like a step forwards, but ideally one that will be reverted in the future as AprLifecycleListener is made more robust. It's difficult to gauge the need for this change prior to a decision being made about that increased robustness.
The only robustness I see is that the client code adds to `Server` only as intended, because connectors are initiated *before* a context and destroyed *after* a context. Context level will still remain wrong. Plus reference counting if there is more than one `Server` in the JVM.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #456:
URL: https://github.com/apache/tomcat/pull/456
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-955175606
> Your test-case looks even more complicated than necessary: just initialize two of them then deinitialize them. No shutdown hook necessary, right?
This maybe true, but it replicates the behavior in Spring Boot. This is what I wanted to show you.
> Okay, so it's not crashing in the `AprLifecycleListener`'s shutdown, which is what it sounded like you were reporting.
I don't think I have actually written that. It is the APR protocol/endpoint.
> Certainly, anything can crash at any time after the APR global pools have been shut-down. We could put guards around those things. Something like this at the top of each of the calls which require APR:
>
> ```
> #DEFINE CHECK_APR_INITIALIZED(ENV) { \
> if(!tcn_global_pool) { \
> tcn_ThrowAPRException((ENV), APR_EINIT); \
> } \
> } \
>
> TCN_IMPLEMENT_CALL(jstring, Address, getnameinfo)(TCN_STDARGS,
> jlong sa, jint flags)
> {
> apr_sockaddr_t *s = J2P(sa, apr_sockaddr_t *);
> char *hostname;
>
> UNREFERENCED(o);
> CHECK_APR_INITIALIZED(e); /* <- This macro invocation is new */
> if (apr_getnameinfo(&hostname, s, (apr_int32_t)flags) == APR_SUCCESS)
> return AJP_TO_JSTRING(hostname);
> else
> return NULL;
> }
> ```
This sounds like a decent idea. It makes post mortem analysis much easier. It is work, but just manual one.
> It would be much cleaner to implement this at the Java level, but as your test-case demonstrates, it's always possible for APR to disappear during the execution of one of the native methods.
Agreed.
> I'm kind of curious as to exactly where in `Socket.accept()` this particular failure occurred. We use APR pools for things like string values, even to throw exceptions. So it's hard to avoid using APR anywhere. One could argue that APR pools aren't necessary for throwing exceptions, but re-writing tcnative at this point to remove APR-type things is unlikely. Perhaps incrementally. But Rémy's recent work with Project Panama looks like we might be able to dump tcnative in the reasonably-near future.
Unlikely since Tomat 9 and 10 will live very long and the minimum Java version is 8. Expect tomcat-native as of now to live for at least five more years. A future version with Panama will likely to have OpenSSL only, for now.
> Anyway, back to fixing this kind of thing:
>
> 1. I think it's worth mentioning the dangers of multiple AprLifecycleListeners in the Tomcat documentation. I don't think the PR as submitted goes far enough. I think it's even misleading at first (e.g. only use `AprLifecycleListener` on a `Server` element) because there are many ways to use the `AprLifecycleListener` in ways that can cause the JVM to crash. I think this documentation should be changed in the Javadoc but also in the documentation for the listener in general, here http://tomcat.apache.org/tomcat-10.0-doc/apr.html#APR_Lifecycle_Listener_Configuration and here http://tomcat.apache.org/tomcat-10.0-doc/config/listeners.html#APR_Lifecycle_Listener_-_org.apache.catalina.core.AprLifecycleListener. (Note that the second of these two already states that the listener should only be used on `<Server>` components.)
This is the first problem, the documentation you mention is intended for integrators and admins. Embedders read Javadoc only, as you have seen with Spring Boot. Wen should add at least minimal information to all of these listeners' Javadoc and maybe a link to the extended documentation. (our fault)
What would be your proposal for the documentation.
> 2. I think it's also reasonable to try to protect the JVM against this kind of failure. Anyone starting two Tomcats in a single JVM is going to have exactly the same problem. Yes, it's possible to have the "container" (e.g. Spring in this case) manage the whole, um, _lifecycle_ of the `AprLifecycleListener` but it's much more natural to use it "inside" Tomcat and allow Tomcat to manage that process.
Yes, natural as long as you adibe and know the contract and requirements which partially did not happen in Spring Boot. I am inclined to add a WARNING log to all of these listeners when the `Lifecycle` is not of instance `Server`.
> I like the idea of reference-counting, especially because the number of times the `AprLifecycleListener` is initialized and de-initialized in a given JVM should be relatively low. It's a small amount of code to manage, provides a great amount of protection (JVM crashes should really never be tolerated), and the performance impact is irrelevant.
>
> Would you care to prepare a reference-counting patch for `AprLifecycleListener` either as a part of this PR, or as a separate one?
I would like to take care of the documentation shortage first and then discuss in a separate PR/thread reference counting approach.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] wilkinsona commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
wilkinsona commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954908926
I agree, @ChristopherSchultz. IMO, the second `AprLifecycleListener` should not be shutting down APR if it did nothing when asked to initialize it.
The ordering in Spring Boot is the following:
1. Start main Tomcat instance
2. Start management Tomcat instance
3. Stop management Tomcat instance
4. Stop main Tomcat instance
From an APR perspective, the following happens at the four steps above:
1. APR is initialized in response to `BEFORE_INIT_EVENT`
2. APR is already initialized so handling of `BEFORE_INIT_EVENT` is essentially a no-op
3. APR is terminated in `AFTER_DESTROY_EVENT`
The JVM crashes before we get to step 4 as APR is been ripped out from underneath the main Tomcat instance.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954959277
@ChristopherSchultz Here is an extremely primitive test case:
```java
public class TomcatEmbeddedTest {
static class ShutdownHook2 extends Thread {
Tomcat tomcat;
Tomcat tomcat2;
public void run() {
try {
Thread.sleep(5000L);
} catch (InterruptedException e1) {
e1.printStackTrace();
}
try {
tomcat2.stop();
tomcat2.destroy();
tomcat.stop();
tomcat.destroy();
} catch (LifecycleException e) {
e.printStackTrace();
}
}
}
public static void main(String[] args) throws LifecycleException {
Tomcat tomcat = new Tomcat();
tomcat.setPort(18888);
AprLifecycleListener listener = new AprLifecycleListener();
listener.setUseAprConnector(true);
listener.setUseOpenSSL(true);
tomcat.getServer().addLifecycleListener(listener);
tomcat.getConnector();
tomcat.init();
Tomcat tomcat2 = new Tomcat();
tomcat2.setPort(28888);
AprLifecycleListener listener2 = new AprLifecycleListener();
listener2.setUseAprConnector(true);
listener2.setUseOpenSSL(true);
tomcat2.getServer().addLifecycleListener(listener2);
tomcat2.getConnector();
tomcat2.init();
ShutdownHook2 sh2 = new ShutdownHook2();
sh2.tomcat = tomcat;
sh2.tomcat2 = tomcat2;
sh2.start();
tomcat.start();
tomcat2.start();
}
}
```
Output:
```
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.AprLifecycleListener lifecycleEvent
INFORMATION: Loaded Apache Tomcat Native library [1.2.31] using APR version [1.7.0].
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.AprLifecycleListener lifecycleEvent
INFORMATION: APR capabilities: IPv6 [true], sendfile [true], accept filters [false], random [true], UDS [true].
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.AprLifecycleListener lifecycleEvent
INFORMATION: APR/OpenSSL configuration: useAprConnector [true], useOpenSSL [true]
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.AprLifecycleListener initializeSSL
INFORMATION: OpenSSL successfully initialized [OpenSSL 1.1.1l 24 Aug 2021]
Okt 29, 2021 8:26:01 PM org.apache.coyote.AbstractProtocol init
INFORMATION: Initializing ProtocolHandler ["http-apr-18888"]
Okt 29, 2021 8:26:01 PM org.apache.coyote.AbstractProtocol init
INFORMATION: Initializing ProtocolHandler ["http-apr-28888"]
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.StandardService startInternal
INFORMATION: Starting service [Tomcat]
Okt 29, 2021 8:26:01 PM org.apache.coyote.AbstractProtocol start
INFORMATION: Starting ProtocolHandler ["http-apr-18888"]
Okt 29, 2021 8:26:01 PM org.apache.catalina.core.StandardService startInternal
INFORMATION: Starting service [Tomcat]
Okt 29, 2021 8:26:01 PM org.apache.coyote.AbstractProtocol start
INFORMATION: Starting ProtocolHandler ["http-apr-28888"]
Okt 29, 2021 8:26:06 PM org.apache.coyote.AbstractProtocol pause
INFORMATION: Pausing ProtocolHandler ["http-apr-28888"]
Okt 29, 2021 8:26:06 PM org.apache.catalina.core.StandardService stopInternal
INFORMATION: Stopping service [Tomcat]
Okt 29, 2021 8:26:06 PM org.apache.coyote.AbstractProtocol stop
INFORMATION: Stopping ProtocolHandler ["http-apr-28888"]
Okt 29, 2021 8:26:06 PM org.apache.coyote.AbstractProtocol destroy
INFORMATION: Destroying ProtocolHandler ["http-apr-28888"]
Okt 29, 2021 8:26:06 PM org.apache.coyote.AbstractProtocol pause
INFORMATION: Pausing ProtocolHandler ["http-apr-18888"]
#
# A fatal error has been detected by the Java Runtime Environment:
#
# EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x00000001800011dd, pid=19692, tid=0x0000000000004138
#
# JRE version: OpenJDK Runtime Environment (Zulu 8.52.0.24-SA-win64) (8.0_282-b08) (build 1.8.0_282-b08)
# Java VM: OpenJDK 64-Bit Server VM (25.282-b08 mixed mode windows-amd64 compressed oops)
# Problematic frame:
# Okt 29, 2021 8:26:06 PM org.apache.tomcat.util.net.Acceptor run
SCHWERWIEGEND: Socket accept failed
org.apache.tomcat.jni.Error: 730004: Ein Blockierungsvorgang wurde durch einen Aufruf von WSACancelBlockingCall unterbrochen.
at org.apache.tomcat.jni.Socket.accept(Native Method)
at org.apache.tomcat.util.net.AprEndpoint.serverSocketAccept(AprEndpoint.java:753)
at org.apache.tomcat.util.net.AprEndpoint.serverSocketAccept(AprEndpoint.java:85)
at org.apache.tomcat.util.net.Acceptor.run(Acceptor.java:106)
at java.lang.Thread.run(Thread.java:748)
C [tcnative-1.dll+0x11dd]
#
# Failed to write core dump. Minidumps are not enabled by default on client versions of Windows
#
# An error report file with more information is saved as:
# C:\Entwicklung\workspace-2020-12\tomcat-embedded-tester\hs_err_pid19692.log
#
# If you would like to submit a bug report, please visit:
# http://www.azulsystems.com/support/
# The crash happened outside the Java Virtual Machine in native code.
# See problematic frame for where to report the bug.
#
```
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-979167233
Merged manually
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954648720
>
>
> This feels like a step forwards, but ideally one that will be reverted in the future as AprLifecycleListener is made more robust. It's difficult to gauge the need for this change prior to a decision being made about that increased robustness.
The only robustness I see is that the client code adds to `Server` only as intended, because connectors are initiated *before* a context and destroyed *after* a context. Context level will still remain wrong. Plus reference counting if there is more than one `Sever` in the JVM.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954898919
>
>
> Something is wrong with the premise, here. I read through [Spring-28472](https://github.com/spring-projects/spring-boot/issues/28472) and looked at the code for `AprLifecycleListener`. `AprLifecycleListener` should not be shutting-down APR twice. In `AprLifecycleListener.lifecycleEvent`, the `Lifecycle.AFTER_DESTROY` handler uses a static lock for cross-thread synchronization and only calls `terminateAPR` if `AprStatus.isAprAvailable` returns `true`. Notably, `terminateAPR` sets `AprStatus.isAprAvailable` to `false`.
>
> I can see a theoretical threading problem because `AprStatus.isAprAvailable` and `AprStatus.setAprAvailable` are not synchronized, but the static class members being set are declared `volatile` and should require threads to reload their values appropriately.
>
> Are you sure it's the shutdown that's causing the failure?
That's not the failure. Please read my description again. The first Tomcat instance starts `AprLifecycleListener`. The scond skips it. Shutdown does reverse order, so the second Tomcat now shuts down APR globally, releasing the global structures. The first Tomcat is now shutting down, its `AprConnector` tries to use a sub APR pool/object from a pool which has already been released.
See
* https://github.com/apache/tomcat-native/blob/c17d3e0e0a594604ff4f30065360aacc688cfb50/native/src/jnilib.c#L243-L262
* https://github.com/apache/tomcat/blob/ae4ee893f99f226fe5fdc8e933313e2e759d0733/java/org/apache/tomcat/jni/Library.java#L102-L109
* https://apr.apache.org/docs/apr/1.6/group__apr__library.html#ga4a91a6b9ff457ead13e670950127761a
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] wilkinsona commented on a change in pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
wilkinsona commented on a change in pull request #456:
URL: https://github.com/apache/tomcat/pull/456#discussion_r739128184
##########
File path: java/org/apache/catalina/core/AprLifecycleListener.java
##########
@@ -38,6 +38,13 @@
/**
* Implementation of <code>LifecycleListener</code> that will init and
* and destroy APR.
+ * <p>
+ * <strong>Note</strong>: This listener must only be used within a {@code Server}
+ * element to manage APR/OpenSSL init and destroy JVM-wide. If you are running
+ * Tomcat in an embedded fashion and have more than one Tomcat instance per JVM,
+ * this listener <em>must not</em> be added to the {@code Server} instance, but
Review comment:
Maybe this is too subtle for Tomcat's javadoc, but this isn't strictly true. You can safely add the listener to a `Server` (or even a `Context` if you know there will only be one) as long as that `Server` is started first and stopped last. In the absence of an API that's designed to manage the lifecycle of APR outside of Tomcat's `Lifecycle` and `LifecycleEvent`, I suspect that's what we'll do in Spring Boot if the Tomcat team decides not to make `AprLifecycleListener` more robust.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954934999
> [...] so the second Tomcat now shuts down APR globally, releasing the global structures. The first Tomcat is now shutting down, its `AprConnector` tries to use a sub APR pool/object from a pool which has already been released.
There is no `AprConnector`. Where is the Java call which eventually goes down into native to cause this problem? As the AprLifecycleListener is stopping, it should not try to use any APR pool. Perhaps the other connector is still trying to actually use pool objects in some other way. If that's the case, the problem isn't with double-shutdown. It's with shutdown-while-trying-to-still-do-stuff.
See
> * https://github.com/apache/tomcat-native/blob/c17d3e0e0a594604ff4f30065360aacc688cfb50/native/src/jnilib.c#L243-L262
Notably, even this native method prevents crashes due to double-shutdown. The predicate around the whole thing prevents it from being called twice.
So can we get a Java stack trace for where this is crashing? It should be simple to check to see if APR has been killed at some point and at least not crash the JVM.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954947125
>
>
> > [...] so the second Tomcat now shuts down APR globally, releasing the global structures. The first Tomcat is now shutting down, its `AprConnector` tries to use a sub APR pool/object from a pool which has already been released.
>
> There is no `AprConnector`.
I actually meant the connector using the APR Protocol with the APR endpoint.
Where is the Java call which eventually goes down into native to cause this problem? As the AprLifecycleListener is stopping, it should not try to use any APR pool. Perhaps the other connector is still trying to actually use pool objects in some other way. If that's the case, the problem isn't with double-shutdown. It's with shutdown-while-trying-to-still-do-stuff.
>
> So can we get a Java stack trace for where this is crashing? It should be simple to check to see if APR has been killed at some point and at least not crash the JVM.
How does this actually matter? If APR has been globally terminated any subsequent operation to any APR object is deemed to fail and that this what happens here. The problem is just init and destroy and wrong points in time. That's it.
```
Stack: [0x00007fffdcccc000,0x00007fffdcdcc000], sp=0x00007fffdcdcae70, free space=1019k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C [libapr-1.so.0+0x336fa] signed char+0x1a
C [libtcnative-1.so.0.2.24+0x19a06] Java_org_apache_tomcat_jni_Address_get+0x36
j org.apache.tomcat.jni.Address.get(IJ)J+0
j org.apache.tomcat.util.net.AprEndpoint.getLocalAddress()Ljava/net/InetSocketAddress;+15
j org.apache.tomcat.util.net.AbstractEndpoint.unlockAccept()V+26
j org.apache.tomcat.util.net.AbstractEndpoint.closeServerSocketGraceful()V+23
j org.apache.coyote.AbstractProtocol.closeServerSocketGraceful()V+4
j org.apache.catalina.core.StandardService.stopInternal()V+35
j org.apache.catalina.util.LifecycleBase.stop()V+214
j org.apache.catalina.core.StandardServer.stopInternal()V+82
j org.apache.catalina.util.LifecycleBase.stop()V+214
j org.apache.catalina.startup.Tomcat.stop()V+9
j org.springframework.boot.web.embedded.tomcat.TomcatWebServer.stopTomcat()V+29
j org.springframework.boot.web.embedded.tomcat.TomcatWebServer.stop()V+32
j org.springframework.boot.web.servlet.context.WebServerStartStopLifecycle.stop()V+4
j org.springframework.context.SmartLifecycle.stop(Ljava/lang/Runnable;)V+1
```
This is from the main Tomcat instance and shutdown time. The management one is already gone.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954890217
Something is wrong with the premise, here. I read through [Spring-28472](https://github.com/spring-projects/spring-boot/issues/28472) and looked at the code for `AprLifecycleListener`. `AprLifecycleListener` should not be shutting-down APR twice. In `AprLifecycleListener.lifecycleEvent`, the `Lifecycle.AFTER_DESTROY` handler uses a static lock for cross-thread synchronization and only calls `terminateAPR` if `AprStatus.isAprAvailable` returns `true`. Notably, `terminateAPR` sets `AprStatus.isAprAvailable` to `false`.
I can see a theoretical threading problem because `AprStatus.isAprAvailable` and `AprStatus.setAprAvailable` are not synchronized, but the static class members being set are declared `volatile` and should require threads to reload their values appropriately.
Are you sure it's the shutdown that's causing the failure?
I think I'd rather fix a bug that seems to be hiding, here, rather than document what can and cannot be done (in a way that is very difficult to understand, I have to admit).
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #456:
URL: https://github.com/apache/tomcat/pull/456#discussion_r739135782
##########
File path: java/org/apache/catalina/core/AprLifecycleListener.java
##########
@@ -38,6 +38,13 @@
/**
* Implementation of <code>LifecycleListener</code> that will init and
* and destroy APR.
+ * <p>
+ * <strong>Note</strong>: This listener must only be used within a {@code Server}
+ * element to manage APR/OpenSSL init and destroy JVM-wide. If you are running
+ * Tomcat in an embedded fashion and have more than one Tomcat instance per JVM,
+ * this listener <em>must not</em> be added to the {@code Server} instance, but
Review comment:
I almost agree with your statement. What would you change in the documentation if the code remains the same?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on pull request #456: Document conditions under which the AprLifecycleListener can be used …
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/tomcat/pull/456#issuecomment-954912903
@wilkinsona Correct. The connector on main Tomcat is still active while APR is already gone.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org