[Bug 68603] Request Context path and Query param gets replaced
https://bz.apache.org/bugzilla/show_bug.cgi?id=68603 Mark Thomas changed: What|Removed |Added Status|REOPENED|NEEDINFO --- Comment #3 from Mark Thomas --- That shouldn't happen. Please provide the simplest possible test case that demonstrates this issue (including source) and we will investigate. -- 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
Re: (tomcat) 04/04: Reduce changes of crash on Library shutdown with OpenSSL connections
On Thu, Feb 8, 2024 at 10:11 PM wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch 10.1.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit e19fa66b8397194f421134debb1f71b590a1f0c0
> Author: Mark Thomas
> AuthorDate: Thu Feb 8 18:54:45 2024 +
>
> Reduce changes of crash on Library shutdown with OpenSSL connections
Do you have a test to reproduce that sort of crash ?
I believe the FFM version is "ok" but if you have some easy procedure
to verify that it would be nice.
Rémy
> ---
> .../tomcat/util/net/openssl/OpenSSLContext.java | 19
> +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
> b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
> index f1d7b092ec..12dc41455b 100644
> --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
> +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
> @@ -46,6 +46,7 @@ import javax.net.ssl.X509TrustManager;
> import org.apache.juli.logging.Log;
> import org.apache.juli.logging.LogFactory;
> import org.apache.tomcat.jni.CertificateVerifier;
> +import org.apache.tomcat.jni.Library;
> import org.apache.tomcat.jni.Pool;
> import org.apache.tomcat.jni.SSL;
> import org.apache.tomcat.jni.SSLConf;
> @@ -648,14 +649,16 @@ public class OpenSSLContext implements
> org.apache.tomcat.util.net.SSLContext {
>
> @Override
> public void run() {
> -if (ctx != 0) {
> -SSLContext.free(ctx);
> -}
> -if (cctx != 0) {
> -SSLConf.free(cctx);
> -}
> -if (aprPool != 0) {
> -Pool.destroy(aprPool);
> +if (Library.isInitialized()) {
> +if (ctx != 0) {
> +SSLContext.free(ctx);
> +}
> +if (cctx != 0) {
> +SSLConf.free(cctx);
> +}
> +if (aprPool != 0) {
> +Pool.destroy(aprPool);
> +}
> }
> }
> }
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: (tomcat) 04/04: Reduce changes of crash on Library shutdown with OpenSSL connections
On 09/02/2024 09:35, Rémy Maucherat wrote:
On Thu, Feb 8, 2024 at 10:11 PM wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e19fa66b8397194f421134debb1f71b590a1f0c0
Author: Mark Thomas
AuthorDate: Thu Feb 8 18:54:45 2024 +
Reduce changes of crash on Library shutdown with OpenSSL connections
Do you have a test to reproduce that sort of crash ?
I believe the FFM version is "ok" but if you have some easy procedure
to verify that it would be nice.
I could only recreate it on 9.0.x and earlier with the
TestSSLHostConfigCompat tests.
I did wonder whether the change was necessary for 10.1.x and 11.0.x. In
the end I opted for consistency but I'm happy to revert the
Library.isInitialized() changes in 10.1.x onwards if that is the consensus.
Mark
Rémy
---
.../tomcat/util/net/openssl/OpenSSLContext.java | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index f1d7b092ec..12dc41455b 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -46,6 +46,7 @@ import javax.net.ssl.X509TrustManager;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.jni.CertificateVerifier;
+import org.apache.tomcat.jni.Library;
import org.apache.tomcat.jni.Pool;
import org.apache.tomcat.jni.SSL;
import org.apache.tomcat.jni.SSLConf;
@@ -648,14 +649,16 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public void run() {
-if (ctx != 0) {
-SSLContext.free(ctx);
-}
-if (cctx != 0) {
-SSLConf.free(cctx);
-}
-if (aprPool != 0) {
-Pool.destroy(aprPool);
+if (Library.isInitialized()) {
+if (ctx != 0) {
+SSLContext.free(ctx);
+}
+if (cctx != 0) {
+SSLConf.free(cctx);
+}
+if (aprPool != 0) {
+Pool.destroy(aprPool);
+}
}
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Test stability and tagging delayed
On 08/02/2024 17:07, Mark Thomas wrote: Hi all, TL;DR tagging likely delayed while APR/native stability issue is addressed We have had a couple of issues with test stability in the last few days. The issues with 11.0.x and 10.1.x were caused by the incomplete convenience binary for Tomcat Native 2.0.7. That should be resolved now. The 11.0.x tests are already green and I am expecting 10.1.x to be green for the next run. 9.0.x and 8.5.x are a little more interesting. The instability was triggered by the change to allow users to provide an SSLContext directly to SSLHostConfigCertificate. This changed the timing of endpoint destruction enough to make the intermittent APR crashes much more frequent - almost on every run. The good news is that the more frequent crashes made it easier to investigate. My current theory is related to the cleanup of OpenSSLContext. In 9.0.x and 8.5.x clean-up of this object is performed by a finalizer. This is to support runtime replacement of the SSLHostContext. What I think happens is: - Tomcat starts shutdown - Endpoint is destroyed - AprLifecycleListener shuts down Native library - finalizer runs and tries to reference native code leading to a crash I have some initial ideas on how we might handle this better. The quick and dirty fix was to force GC and add a delay in AprLifecycleListener.terminateAPR() but that was just a hack to test the theory. Back to working out a more robust fix... While the fix worked well locally, it hasn't fixed the problem on the Buildbot CI worker. I'm going to take another look. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Test stability and tagging delayed
On Fri, Feb 9, 2024 at 12:46 PM Mark Thomas wrote: > > On 08/02/2024 17:07, Mark Thomas wrote: > > Hi all, > > > > TL;DR tagging likely delayed while APR/native stability issue is addressed > > > > We have had a couple of issues with test stability in the last few days. > > > > The issues with 11.0.x and 10.1.x were caused by the incomplete > > convenience binary for Tomcat Native 2.0.7. That should be resolved now. > > The 11.0.x tests are already green and I am expecting 10.1.x to be green > > for the next run. > > > > 9.0.x and 8.5.x are a little more interesting. The instability was > > triggered by the change to allow users to provide an SSLContext directly > > to SSLHostConfigCertificate. This changed the timing of endpoint > > destruction enough to make the intermittent APR crashes much more > > frequent - almost on every run. > > > > The good news is that the more frequent crashes made it easier to > > investigate. My current theory is related to the cleanup of > > OpenSSLContext. In 9.0.x and 8.5.x clean-up of this object is performed > > by a finalizer. This is to support runtime replacement of the > > SSLHostContext. > > > > What I think happens is: > > - Tomcat starts shutdown > > - Endpoint is destroyed > > - AprLifecycleListener shuts down Native library > > - finalizer runs and tries to reference native code leading to a crash > > > > I have some initial ideas on how we might handle this better. The quick > > and dirty fix was to force GC and add a delay in > > AprLifecycleListener.terminateAPR() but that was just a hack to test the > > theory. > > > > Back to working out a more robust fix... > > While the fix worked well locally, it hasn't fixed the problem on the > Buildbot CI worker. > > I'm going to take another look. We don't really have to backport everything immediately, so maybe skip the backport for https://github.com/apache/tomcat/pull/673 for now ? I'm not a fan of these kinds of changes anyway, since this is effectively piling up behavioral requirements and "regressions" simply to allow more hacking potential. Unless of course it is for my own hacking, in which case it is totally fine :D Rémy - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Test stability and tagging delayed
On Fri, Feb 9, 2024 at 12:46 PM Mark Thomas wrote: > > On 08/02/2024 17:07, Mark Thomas wrote: > > Hi all, > > > > TL;DR tagging likely delayed while APR/native stability issue is addressed > > > > We have had a couple of issues with test stability in the last few days. > > > > The issues with 11.0.x and 10.1.x were caused by the incomplete > > convenience binary for Tomcat Native 2.0.7. That should be resolved now. > > The 11.0.x tests are already green and I am expecting 10.1.x to be green > > for the next run. > > > > 9.0.x and 8.5.x are a little more interesting. The instability was > > triggered by the change to allow users to provide an SSLContext directly > > to SSLHostConfigCertificate. This changed the timing of endpoint > > destruction enough to make the intermittent APR crashes much more > > frequent - almost on every run. > > > > The good news is that the more frequent crashes made it easier to > > investigate. My current theory is related to the cleanup of > > OpenSSLContext. In 9.0.x and 8.5.x clean-up of this object is performed > > by a finalizer. This is to support runtime replacement of the > > SSLHostContext. > > > > What I think happens is: > > - Tomcat starts shutdown > > - Endpoint is destroyed > > - AprLifecycleListener shuts down Native library > > - finalizer runs and tries to reference native code leading to a crash > > > > I have some initial ideas on how we might handle this better. The quick > > and dirty fix was to force GC and add a delay in > > AprLifecycleListener.terminateAPR() but that was just a hack to test the > > theory. > > > > Back to working out a more robust fix... > > While the fix worked well locally, it hasn't fixed the problem on the > Buildbot CI worker. > > I'm going to take another look. I had a look at the test output, and the issue is exclusively with the APR connector (the tests are a bit weird so that the APR connector is also run, basically the test is the same for all connectors), that's why it would only affect 8.5 and 9.0. Overall it's not even certain OpenSSL + NIO really needed a fix and the OpenSSLContext cleanup is most likely good enough (at least in that case). Build 845 for 9.0.x was crashing when using the APR connector, not NIO. Rémy - 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: Should fix CI running TestSSLHostConfigCompat
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 4cc023ff17 Should fix CI running TestSSLHostConfigCompat 4cc023ff17 is described below commit 4cc023ff17f874ac653d6731106dd4b062686d15 Author: remm AuthorDate: Fri Feb 9 14:16:31 2024 +0100 Should fix CI running TestSSLHostConfigCompat --- java/org/apache/tomcat/util/net/AprEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 7f85783590..b7fcad831d 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -479,7 +479,7 @@ public class AprEndpoint extends AbstractEndpoint implements SNICallB sslContext.addCertificate(certificate); } -certificate.setSslContext(sslContext); +certificate.setSslContextGenerated(sslContext); logCertificate(certificate); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Test stability and tagging delayed
On 09/02/2024 12:14, Rémy Maucherat wrote: On Fri, Feb 9, 2024 at 12:46 PM Mark Thomas wrote: On 08/02/2024 17:07, Mark Thomas wrote: Back to working out a more robust fix... While the fix worked well locally, it hasn't fixed the problem on the Buildbot CI worker. I'm going to take another look. I had a look at the test output, and the issue is exclusively with the APR connector (the tests are a bit weird so that the APR connector is also run, basically the test is the same for all connectors), that's why it would only affect 8.5 and 9.0. Overall it's not even certain OpenSSL + NIO really needed a fix and the OpenSSLContext cleanup is most likely good enough (at least in that case). Build 845 for 9.0.x was crashing when using the APR connector, not NIO. I am making progress. I have a slightly amended theory as to what is going on (and now confirmed via logging). The clean-up of OpenSSLContext is performed by the finalizer. In the problematic tests, the Native library is repeatedly loaded and unloaded within a single JVM. The crashes happen when the finalizer attempts to clean an OpenSSLContext instance from a previous load of the native library after the native library has been reloaded. I have a fix for this. I intend to: - revert (most of) the changes made to 8.5.x through 11.0.x - apply the new fix to 9.0.x and 8.5.x only Apologies for the noise while I tracked the root cause of this down. The good news is that this should address one of the causes of unit test instability. Mark - 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.0.x
Build status: Build succeeded! Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/37/builds/851 Blamelist: Mark Thomas , remm Build Text: build successful Status Detected: restored build Build Source Stamp: [branch 9.0.x] 4cc023ff17f874ac653d6731106dd4b062686d15 Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 1 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 8.5.x updated: Should fix CI running TestSSLHostConfigCompat
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 517e0966eb Should fix CI running TestSSLHostConfigCompat 517e0966eb is described below commit 517e0966eb438cd3e2cd41e178c17933fbb855ab Author: remm AuthorDate: Fri Feb 9 14:16:31 2024 +0100 Should fix CI running TestSSLHostConfigCompat --- java/org/apache/tomcat/util/net/AprEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 6aafe6ac97..e8fb34cb83 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -413,7 +413,7 @@ public class AprEndpoint extends AbstractEndpoint implements SNICallB sslContext.addCertificate(certificate); } -certificate.setSslContext(sslContext); +certificate.setSslContextGenerated(sslContext); logCertificate(certificate); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 10.1.x updated: Revert most of the changes for Native library stability
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new b2fbf15e35 Revert most of the changes for Native library stability
b2fbf15e35 is described below
commit b2fbf15e3572ff5ffd16d68346d129e09fd11e9a
Author: Mark Thomas
AuthorDate: Fri Feb 9 14:06:57 2024 +
Revert most of the changes for Native library stability
Further investigation indicates these changes were not necessary
---
.../apache/catalina/core/AprLifecycleListener.java | 3 +--
java/org/apache/tomcat/jni/Library.java| 23 +-
.../tomcat/util/net/openssl/OpenSSLContext.java| 19 --
test/org/apache/tomcat/util/net/TesterSupport.java | 1 -
webapps/docs/changelog.xml | 4
5 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java
b/java/org/apache/catalina/core/AprLifecycleListener.java
index 8fea80148e..f1621ff7e7 100644
--- a/java/org/apache/catalina/core/AprLifecycleListener.java
+++ b/java/org/apache/catalina/core/AprLifecycleListener.java
@@ -169,11 +169,10 @@ public class AprLifecycleListener implements
LifecycleListener {
}
private static void terminateAPR() {
-Library.terminatePrepare();
AprStatus.setAprInitialized(false);
AprStatus.setAprAvailable(false);
fipsModeActive = false;
-sslInitialized = false; // Well we cleaned the pool in terminate.
+sslInitialized = false; // terminate() will clean the pool
Library.terminate();
}
diff --git a/java/org/apache/tomcat/jni/Library.java
b/java/org/apache/tomcat/jni/Library.java
index 590f1b19d9..78a0bfd39e 100644
--- a/java/org/apache/tomcat/jni/Library.java
+++ b/java/org/apache/tomcat/jni/Library.java
@@ -29,8 +29,6 @@ public final class Library {
*/
private static Library _instance = null;
-private static volatile boolean initialized = false;
-
private Library() throws Exception {
boolean loaded = false;
StringBuilder err = new StringBuilder();
@@ -103,22 +101,9 @@ public final class Library {
* Create Tomcat Native's global APR pool. This has to be the first call
to TCN library.
*/
private static native boolean initialize();
-/**
- * Signal that Tomcat Native is about to be shutdown.
- *
- * The main purpose of this flag is to allow instances that manage their
own APR root pools to determine if those
- * pools need to be explicitly cleaned up or if they will be / have been
cleaned up by the call to
- * {@link #terminate()}. The code needs to avoid multiple attempts to
clean up these pools else the Native code may
- * crash.
- */
-public static void terminatePrepare() {
-initialized = false;
-}
/**
* Destroys Tomcat Native's global APR pool. This has to be the last call
to TCN library. This will destroy any APR
* root pools that have not been explicitly destroyed.
- *
- * Callers of this method should call {@link #terminatePrepare()} before
calling this method.
*/
public static native void terminate();
/* Internal function for loading APR Features */
@@ -177,12 +162,6 @@ public final class Library {
aprVersionString() + ")");
}
}
-initialized = initialize();
-return initialized;
-}
-
-
-public static boolean isInitialized() {
-return initialized;
+return initialize();
}
}
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index 12dc41455b..f1d7b092ec 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -46,7 +46,6 @@ import javax.net.ssl.X509TrustManager;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.jni.CertificateVerifier;
-import org.apache.tomcat.jni.Library;
import org.apache.tomcat.jni.Pool;
import org.apache.tomcat.jni.SSL;
import org.apache.tomcat.jni.SSLConf;
@@ -649,16 +648,14 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public void run() {
-if (Library.isInitialized()) {
-if (ctx != 0) {
-SSLContext.free(ctx);
-}
-if (cctx != 0) {
-SSLConf.free(cctx);
-}
-if (aprPool != 0) {
-Pool.destroy(aprPool);
-}
+if (ctx != 0) {
+SSLContext.free(ctx);
+}
+if (cctx != 0) {
+
(tomcat) branch main updated: Revert most of the changes for Native library stability
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 587389580e Revert most of the changes for Native library stability
587389580e is described below
commit 587389580eefeefe4c1a0f1aa676fcd051d27e8c
Author: Mark Thomas
AuthorDate: Fri Feb 9 14:06:57 2024 +
Revert most of the changes for Native library stability
Further investigation indicates these changes were not necessary
---
.../apache/catalina/core/AprLifecycleListener.java | 3 +--
java/org/apache/tomcat/jni/Library.java| 23 +-
.../tomcat/util/net/openssl/OpenSSLContext.java| 19 --
test/org/apache/tomcat/util/net/TesterSupport.java | 1 -
webapps/docs/changelog.xml | 4
5 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java
b/java/org/apache/catalina/core/AprLifecycleListener.java
index 8fea80148e..f1621ff7e7 100644
--- a/java/org/apache/catalina/core/AprLifecycleListener.java
+++ b/java/org/apache/catalina/core/AprLifecycleListener.java
@@ -169,11 +169,10 @@ public class AprLifecycleListener implements
LifecycleListener {
}
private static void terminateAPR() {
-Library.terminatePrepare();
AprStatus.setAprInitialized(false);
AprStatus.setAprAvailable(false);
fipsModeActive = false;
-sslInitialized = false; // Well we cleaned the pool in terminate.
+sslInitialized = false; // terminate() will clean the pool
Library.terminate();
}
diff --git a/java/org/apache/tomcat/jni/Library.java
b/java/org/apache/tomcat/jni/Library.java
index 908a9a8b56..a7686c0935 100644
--- a/java/org/apache/tomcat/jni/Library.java
+++ b/java/org/apache/tomcat/jni/Library.java
@@ -29,8 +29,6 @@ public final class Library {
*/
private static Library _instance = null;
-private static volatile boolean initialized = false;
-
private Library() throws Exception {
boolean loaded = false;
StringBuilder err = new StringBuilder();
@@ -103,22 +101,9 @@ public final class Library {
* Create Tomcat Native's global APR pool. This has to be the first call
to TCN library.
*/
private static native boolean initialize();
-/**
- * Signal that Tomcat Native is about to be shutdown.
- *
- * The main purpose of this flag is to allow instances that manage their
own APR root pools to determine if those
- * pools need to be explicitly cleaned up or if they will be / have been
cleaned up by the call to
- * {@link #terminate()}. The code needs to avoid multiple attempts to
clean up these pools else the Native code may
- * crash.
- */
-public static void terminatePrepare() {
-initialized = false;
-}
/**
* Destroys Tomcat Native's global APR pool. This has to be the last call
to TCN library. This will destroy any APR
* root pools that have not been explicitly destroyed.
- *
- * Callers of this method should call {@link #terminatePrepare()} before
calling this method.
*/
public static native void terminate();
/* Internal function for loading APR Features */
@@ -177,12 +162,6 @@ public final class Library {
aprVersionString() + ")");
}
}
-initialized = initialize();
-return initialized;
-}
-
-
-public static boolean isInitialized() {
-return initialized;
+return initialize();
}
}
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index 12dc41455b..f1d7b092ec 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -46,7 +46,6 @@ import javax.net.ssl.X509TrustManager;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.jni.CertificateVerifier;
-import org.apache.tomcat.jni.Library;
import org.apache.tomcat.jni.Pool;
import org.apache.tomcat.jni.SSL;
import org.apache.tomcat.jni.SSLConf;
@@ -649,16 +648,14 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public void run() {
-if (Library.isInitialized()) {
-if (ctx != 0) {
-SSLContext.free(ctx);
-}
-if (cctx != 0) {
-SSLConf.free(cctx);
-}
-if (aprPool != 0) {
-Pool.destroy(aprPool);
-}
+if (ctx != 0) {
+SSLContext.free(ctx);
+}
+if (cctx != 0) {
+
Re: (tomcat) branch main updated: Jextract library load refinements
On Thu, Feb 8, 2024 at 4:30 PM wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new 830771823e Jextract library load refinements
> 830771823e is described below
>
> commit 830771823edbb79d2884eb1c6f5122e81df4df78
> Author: remm
> AuthorDate: Thu Feb 8 16:30:07 2024 +0100
>
> Jextract library load refinements
This makes OpenSSL FFM work on the CI runs, so it actually does something ;)
Rémy
> ---
> java/org/apache/tomcat/util/openssl/openssl_h.java | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/openssl/openssl_h.java
> b/java/org/apache/tomcat/util/openssl/openssl_h.java
> index f006407e23..25ba1ae96f 100644
> --- a/java/org/apache/tomcat/util/openssl/openssl_h.java
> +++ b/java/org/apache/tomcat/util/openssl/openssl_h.java
> @@ -29,13 +29,6 @@ import static java.lang.foreign.ValueLayout.*;
> @SuppressWarnings({"javadoc", "boxing"})
> public class openssl_h {
>
> -static final SymbolLookup SYMBOL_LOOKUP
> -=
> SymbolLookup.loaderLookup().or(Linker.nativeLinker().defaultLookup());
> -
> -static {
> -System.loadLibrary("ssl");
> -}
> -
> openssl_h() {
> // Suppresses public default constructor, ensuring
> non-instantiability,
> // but allows generated subclasses in same package.
> @@ -54,6 +47,9 @@ public class openssl_h {
>
> static final Arena LIBRARY_ARENA = Arena.ofAuto();
> static final boolean TRACE_DOWNCALLS =
> Boolean.getBoolean("jextract.trace.downcalls");
> +static final SymbolLookup SYMBOL_LOOKUP =
> SymbolLookup.libraryLookup(System.mapLibraryName("ssl"), LIBRARY_ARENA)
> +.or(SymbolLookup.loaderLookup())
> +.or(Linker.nativeLinker().defaultLookup());
>
> static void traceDowncall(String name, Object... args) {
> String traceArgs = Arrays.stream(args)
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
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: Rework Native library stability fix.
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 b06eb25ffa Rework Native library stability fix.
b06eb25ffa is described below
commit b06eb25ffa6cc7bb25611de19ac9e5e287815c49
Author: Mark Thomas
AuthorDate: Fri Feb 9 12:35:59 2024 +
Rework Native library stability fix.
There are two aspects to this:
- Don't let the library temerinate while finalize is clearing
OpenSSLContext instances
- Don't allow finalize to clear OpenSSLContext instances once the
library generaiton that created that instance has terminated
---
.../apache/catalina/core/AprLifecycleListener.java | 5 +-
java/org/apache/tomcat/jni/Library.java| 53 --
.../tomcat/util/net/openssl/OpenSSLContext.java| 26 +++
test/org/apache/tomcat/util/net/TesterSupport.java | 1 -
4 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java
b/java/org/apache/catalina/core/AprLifecycleListener.java
index 974af71ba2..498e5179d2 100644
--- a/java/org/apache/catalina/core/AprLifecycleListener.java
+++ b/java/org/apache/catalina/core/AprLifecycleListener.java
@@ -169,14 +169,11 @@ public class AprLifecycleListener implements
LifecycleListener {
}
private static void terminateAPR() {
-Library.terminatePrepare();
-// Need to force GC here as some components do APR clean-up in
finalize()
-System.gc();
AprStatus.setAprInitialized(false);
AprStatus.setAprAvailable(false);
fipsModeActive = false;
sslInitialized = false; // Well we cleaned the pool in terminate.
-Library.terminate();
+Library.threadSafeTerminate();
}
@SuppressWarnings("deprecation")
diff --git a/java/org/apache/tomcat/jni/Library.java
b/java/org/apache/tomcat/jni/Library.java
index 2576beecd1..c8e62f4a27 100644
--- a/java/org/apache/tomcat/jni/Library.java
+++ b/java/org/apache/tomcat/jni/Library.java
@@ -17,6 +17,10 @@
package org.apache.tomcat.jni;
import java.io.File;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
public final class Library {
@@ -32,7 +36,8 @@ public final class Library {
*/
private static Library _instance = null;
-private static volatile boolean initialized = false;
+private static final AtomicLong generation = new AtomicLong(0);
+private static final ReadWriteLock cleanUpLock = new
ReentrantReadWriteLock();
private Library() throws Exception {
boolean loaded = false;
@@ -107,21 +112,25 @@ public final class Library {
*/
private static native boolean initialize();
/**
- * Signal that Tomcat Native is about to be shutdown.
- *
- * The main purpose of this flag is to allow instances that manage their
own APR root pools to determine if those
- * pools need to be explicitly cleaned up or if they will be / have been
cleaned up by the call to
- * {@link #terminate()}. The code needs to avoid multiple attempts to
clean up these pools else the Native code may
- * crash.
+ * Allows for thread safe termination when other threads may be attempting
clean-up concurrently with the current
+ * thread. Waits for any threads currently holding the clean-up lock to
release the lock and then calls
+ * {@link #terminate()}.
*/
-public static void terminatePrepare() {
-initialized = false;
+public static void threadSafeTerminate() {
+cleanUpLock.writeLock().lock();
+try {
+terminate();
+} finally {
+generation.incrementAndGet();
+cleanUpLock.writeLock().unlock();
+}
}
/**
* Destroys Tomcat Native's global APR pool. This has to be the last call
to TCN library. This will destroy any APR
* root pools that have not been explicitly destroyed.
*
- * Callers of this method should call {@link #terminatePrepare()} before
calling this method.
+ * This method should only be used if the caller is certain that all other
threads have finished using the native
+ * library.
*/
public static native void terminate();
/* Internal function for loading APR Features */
@@ -300,13 +309,29 @@ public final class Library {
throw new UnsatisfiedLinkError("Missing threading support from
APR");
}
}
-initialized = initialize();
-return initialized;
+return initialize();
+}
+
+
+public static boolean tryCleanUpLock(long cleanupGeneration) {
+try {
+boolean result = cleanUpLock.readLock().tryLock(0,
(tomcat) branch 8.5.x updated: Rework Native library stability fix.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new ae448b3461 Rework Native library stability fix.
ae448b3461 is described below
commit ae448b346135f381ff695c16dd4a24bac184a3e5
Author: Mark Thomas
AuthorDate: Fri Feb 9 12:35:59 2024 +
Rework Native library stability fix.
There are two aspects to this:
- Don't let the library temerinate while finalize is clearing
OpenSSLContext instances
- Don't allow finalize to clear OpenSSLContext instances once the
library generaiton that created that instance has terminated
---
.../apache/catalina/core/AprLifecycleListener.java | 5 +-
java/org/apache/tomcat/jni/Library.java| 53 --
.../tomcat/util/net/openssl/OpenSSLContext.java| 26 +++
test/org/apache/tomcat/util/net/TesterSupport.java | 1 -
4 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/java/org/apache/catalina/core/AprLifecycleListener.java
b/java/org/apache/catalina/core/AprLifecycleListener.java
index ffcc694e6f..604d01502a 100644
--- a/java/org/apache/catalina/core/AprLifecycleListener.java
+++ b/java/org/apache/catalina/core/AprLifecycleListener.java
@@ -175,14 +175,11 @@ public class AprLifecycleListener implements
LifecycleListener {
}
private static void terminateAPR() {
-Library.terminatePrepare();
-// Need to force GC here as some components do APR clean-up in
finalize()
-System.gc();
aprInitialized = false;
aprAvailable = false;
fipsModeActive = false;
sslInitialized = false; // Well we cleaned the pool in terminate.
-Library.terminate();
+Library.threadSafeTerminate();
}
@SuppressWarnings("deprecation")
diff --git a/java/org/apache/tomcat/jni/Library.java
b/java/org/apache/tomcat/jni/Library.java
index a08dae4c07..e876c13d73 100644
--- a/java/org/apache/tomcat/jni/Library.java
+++ b/java/org/apache/tomcat/jni/Library.java
@@ -17,6 +17,10 @@
package org.apache.tomcat.jni;
import java.io.File;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
public final class Library {
@@ -30,7 +34,8 @@ public final class Library {
*/
private static Library _instance = null;
-private static volatile boolean initialized = false;
+private static final AtomicLong generation = new AtomicLong(0);
+private static final ReadWriteLock cleanUpLock = new
ReentrantReadWriteLock();
private Library() throws Exception {
boolean loaded = false;
@@ -81,21 +86,25 @@ public final class Library {
*/
private static native boolean initialize();
/**
- * Signal that Tomcat Native is about to be shutdown.
- *
- * The main purpose of this flag is to allow instances that manage their
own APR root pools to determine if those
- * pools need to be explicitly cleaned up or if they will be / have been
cleaned up by the call to
- * {@link #terminate()}. The code needs to avoid multiple attempts to
clean up these pools else the Native code may
- * crash.
+ * Allows for thread safe termination when other threads may be attempting
clean-up concurrently with the current
+ * thread. Waits for any threads currently holding the clean-up lock to
release the lock and then calls
+ * {@link #terminate()}.
*/
-public static void terminatePrepare() {
-initialized = false;
+public static void threadSafeTerminate() {
+cleanUpLock.writeLock().lock();
+try {
+terminate();
+} finally {
+generation.incrementAndGet();
+cleanUpLock.writeLock().unlock();
+}
}
/**
* Destroys Tomcat Native's global APR pool. This has to be the last call
to TCN library. This will destroy any APR
* root pools that have not been explicitly destroyed.
*
- * Callers of this method should call {@link #terminatePrepare()} before
calling this method.
+ * This method should only be used if the caller is certain that all other
threads have finished using the native
+ * library.
*/
public static native void terminate();
/* Internal function for loading APR Features */
@@ -263,13 +272,29 @@ public final class Library {
throw new UnsatisfiedLinkError("Missing threading support from
APR");
}
}
-initialized = initialize();
-return initialized;
+return initialize();
+}
+
+
+public static boolean tryCleanUpLock(long cleanupGeneration) {
+try {
+boolean result = cleanUpLock.readLock().tryLock(0,
TimeUnit.SECONDS);
+
Buildbot success in on tomcat-8.5.x
Build status: Build succeeded! Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/36/builds/748 Blamelist: Mark Thomas , remm Build Text: build successful Status Detected: restored build Build Source Stamp: [branch 8.5.x] 517e0966eb438cd3e2cd41e178c17933fbb855ab Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 1 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 68596] Remaining overhead in javax.el.CompositeELResolver.convertToType
https://bz.apache.org/bugzilla/show_bug.cgi?id=68596 Mark Thomas changed: What|Removed |Added Severity|normal |enhancement --- Comment #4 from Mark Thomas --- Moving to enhancement for now but I suspect this is going to end up as WONTFIX. -- 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 68596] Remaining overhead in javax.el.CompositeELResolver.convertToType
https://bz.apache.org/bugzilla/show_bug.cgi?id=68596 --- Comment #5 from John Engebretson --- Yep. Thanks! -- 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
