[GitHub] olafk opened a new pull request #1: fixed typos
olafk opened a new pull request #1: fixed typos URL: https://github.com/apache/tomcat-training/pull/1 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] markt-asf closed pull request #1: fixed typos
markt-asf closed pull request #1: fixed typos URL: https://github.com/apache/tomcat-training/pull/1 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/modules/performance-01.html b/modules/performance-01.html index 9fa5655..20171d3 100644 --- a/modules/performance-01.html +++ b/modules/performance-01.html @@ -52,7 +52,7 @@ Class storage Metaspace: Java 8 and later Stores classes Control size with XX:MaxMetaspaceSize - Unlimted by default + Unlimited by default Uses as much memory as the OS will give it diff --git a/modules/tls-01.html b/modules/tls-01.html index a3c7deb..0f5afea 100644 --- a/modules/tls-01.html +++ b/modules/tls-01.html @@ -177,7 +177,7 @@ Step 10: ChangeCipherSpec Client switches to encrypted mode Algorithms agreed in step 2 -Synchronous encryption with MS +Symmetric encryption with MS Client sends message to server @@ -204,7 +204,7 @@ Step 12: ChangeCipherSpec Server switches to encrypted mode Algorithms agreed in step 2 -Synchronous encryption with MS +Symmetric encryption with MS Server sends message to client diff --git a/modules/tls-03.html b/modules/tls-03.html index 1b84cce..6af6201 100644 --- a/modules/tls-03.html +++ b/modules/tls-03.html @@ -80,7 +80,7 @@ Which format? Tomcat 8.5.x or 9.0.x NIO or NIO2 JSSE or OpenSSL implementation - JSSE or OpnSSL configuration (can't mix) + JSSE or OpenSSL configuration (can't mix) Keystore, PKCS12 (JSSE config) PEM (OpenSSL config) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] markt-asf commented on issue #1: fixed typos
markt-asf commented on issue #1: fixed typos URL: https://github.com/apache/tomcat-training/pull/1#issuecomment-380584325 Thanks for this. I only spotted one of these. Much appreciated. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o closed pull request #151: Fix wrong protocol version usage
michael-o closed pull request #151: Fix wrong protocol version usage URL: https://github.com/apache/tomcat/pull/151 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on issue #151: Fix wrong protocol version usage
michael-o commented on issue #151: Fix wrong protocol version usage URL: https://github.com/apache/tomcat/pull/151#issuecomment-478929533 As per discussion, closing this 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] paplorinc commented on a change in pull request #143: Apply deduplication to certain loaded and created Strings
paplorinc commented on a change in pull request #143: Apply deduplication to
certain loaded and created Strings
URL: https://github.com/apache/tomcat/pull/143#discussion_r269449223
##
File path: java/org/apache/tomcat/util/digester/SetPropertiesRule.java
##
@@ -62,7 +59,7 @@ public void begin(String namespace, String theName,
Attributes attributes)
if ("".equals(name)) {
name = attributes.getQName(i);
}
-String value = attributes.getValue(i);
+String value = attributes.getValue(i).intern();
Review comment:
FYI: https://shipilev.net/jvm/anatomy-quarks/10-string-intern
> In almost every project we were taking care of, removing String.intern()
from the hotpaths, or optionally replacing it with a handrolled deduplicator,
was the very profitable performance optimization. Do not use String.intern()
without thinking very hard about it, okay?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #149: Adding ReDoS warning/documentation to RewriteValve
markt-asf commented on issue #149: Adding ReDoS warning/documentation to RewriteValve URL: https://github.com/apache/tomcat/pull/149#issuecomment-475172395 That page is generated from this file: https://github.com/apache/tomcat/blob/master/webapps/docs/rewrite.xml This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10
markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-475197867 Hmm. If you pass "12" or "13" directly to ECJ it silently switches to the latest version it knows - currently 11. Whereas Tomcat would log a warning and switch to the minimum Java version required by the spec (Java 8 for Tomcat 9). Applying this patch would mean "12" was equivalent to "11" until ECJ supports Java 12. The silent switching makes me uneasy. I'm going to see if there is a way to get at the version actually being used. From that we could generate a suitable warning. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10
tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-475179366 Hi I have update the code, and squash commit Tell me if it's ok for you thx This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10
markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-475214511 Thanks. I took your patch and extended to cover Java 13 as well and added a check to log a warning if an unsupported version was used and ECJ fell back to the latest. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #150: Add compiler source and target support for all Java versions over 10
markt-asf closed pull request #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #139: service.bat handles changed Service at installation
markt-asf commented on issue #139: service.bat handles changed Service at installation URL: https://github.com/apache/tomcat/pull/139#issuecomment-476164040 Re-open PR that was closed as a side effect of the svn -> git migration. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] bkiselka opened a new pull request #139: service.bat handles changed Service at installation
bkiselka opened a new pull request #139: service.bat handles changed Service at installation URL: https://github.com/apache/tomcat/pull/139 Enable service.bat to deal with changed "Windows Service Name" defined during Tomcat Windows Installation. During the Apache Tomcat Setup You can define on the "Configuration Options" page a "Windows Service Name". If you change the default "Tomcat8" this results not only in a changed installation folder and a changed Windows Service Name, but also to renamed files bin/Tomcat8.exe and bin/Tomcat8w.exe. The proposed changes deal with renamed Windows service wrapper executables by using the provided parameter name for the service name. Note that the parameter is no longer optional then! Another solution would be to change the Windows installer and do not rename the bin/Tomcat@VERSION_MAJOR@.exe and bin/Tomcat@version_ma...@w.exe during installation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #139: service.bat handles changed Service at installation
markt-asf commented on issue #139: service.bat handles changed Service at installation URL: https://github.com/apache/tomcat/pull/139#issuecomment-476243775 For reference, this was the trigger for the filename to change: https://bz.apache.org/bugzilla/show_bug.cgi?id=50949 The advantage of changing the file name is that Commons Daemon derives the current service name from it so it doesn't have to be explicitly specified. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher opened a new pull request #152: Add async IO API to NIO
rmaucher opened a new pull request #152: Add async IO API to NIO URL: https://github.com/apache/tomcat/pull/152 It has very small perf improvements (except for "sendfile" situations) and non blocking frame read for HTTP/2. The performance looks rather similar to NIO2 as well, it is hard to find a very meaningful difference in my basic tests. The main downside is the two semaphores added. This should allow actual use of the API by users (as long as they accept to give up on the APR connector), and also using it in Tomcat for HTTP/3 once NIO UDP is added. Note: no doc, new "useAsyncIO" flag on the connector (default to false). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #152: Add async IO API to NIO
rmaucher commented on issue #152: Add async IO API to NIO URL: https://github.com/apache/tomcat/pull/152#issuecomment-479919216 Note: I will only merge this after 9.0.18. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher closed pull request #152: Add async IO API to NIO
rmaucher closed pull request #152: Add async IO API to NIO URL: https://github.com/apache/tomcat/pull/152 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher opened a new pull request #153: Add async API for NIO
rmaucher opened a new pull request #153: Add async API for NIO URL: https://github.com/apache/tomcat/pull/153 Includes NIO vectored IO calls. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o opened a new pull request #151: Fix wrong protocol version usage
michael-o opened a new pull request #151: Fix wrong protocol version usage URL: https://github.com/apache/tomcat/pull/151 When serving a HTTP/2 request the protocol version was set as "HTTP/2.0" which does not exist. This needs also to be backported to 8.5 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kkolinko commented on issue #151: Fix wrong protocol version usage
kkolinko commented on issue #151: Fix wrong protocol version usage URL: https://github.com/apache/tomcat/pull/151#issuecomment-478339076 -1 (veto). This was discussed several years ago, and the decision was to use "HTTP/2.0" https://bz.apache.org/bugzilla/show_bug.cgi?id=58605 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #149: Adding ReDoS warning/documentation to RewriteValve
markt-asf closed pull request #149: Adding ReDoS warning/documentation to RewriteValve URL: https://github.com/apache/tomcat/pull/149 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #149: Adding ReDoS warning/documentation to RewriteValve
markt-asf commented on issue #149: Adding ReDoS warning/documentation to RewriteValve URL: https://github.com/apache/tomcat/pull/149#issuecomment-480629451 Thanks. I applied the patch along with a couple of formatting tweaks, a cross-reference from the security how to page and a change log entry (giving you credit for the change). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] mlem commented on issue #154: Don't serialize the whole valve into session, but use a smaller object.
mlem commented on issue #154: Don't serialize the whole valve into session, but use a smaller object. URL: https://github.com/apache/tomcat/pull/154#issuecomment-480863047 I've filed an issue: https://bz.apache.org/bugzilla/show_bug.cgi?id=63324 The problem is, that we serialize sessions between tomcat instances and the CrawlerSessionManagerValve has dependencies on unwanted content (like JDK internals) About the understanding: Yes, it doesn't implement serializable, the library we are using is doing it for us. Even if you don't provide a serializable interface, we can serialize it. I wanted to write a private inner class accessing your maps directly, but I think, that the scope of this class would contain the outer class (not so with private static inner classes). This would bring us the desired effect. That's why I'm passing in the maps. They still exist just once in memory, they are not copies, they are references to the original maps. There are some tricks in java 8 to pass in functions to retrieve the correct maps, but this looks more like a hack than what I did. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] mlem opened a new pull request #154: Don't serialize the whole valve into session, but use a smaller object.
mlem opened a new pull request #154: Don't serialize the whole valve into session, but use a smaller object. URL: https://github.com/apache/tomcat/pull/154 It helps to serialize this sessions with libraries like kryo. Background: we've found out, that our kryo library serializes the whole valve. Our stack indicates, that jdk internals are getting serialized into the sessions. It's unnecessary and can be solves more easily with this fix. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] mlem commented on issue #154: Don't serialize the whole valve into session, but use a smaller object.
mlem commented on issue #154: Don't serialize the whole valve into session, but use a smaller object. URL: https://github.com/apache/tomcat/pull/154#issuecomment-480842434 We are getting this message in our log all the time. I've found out, that our serialization library (Kryo) is serializing your valve into the session and is causing troubles with OpenJDK 11, because it wants to access a private field of the class loader ``` 28-Mar-2019 11:30:11.530 WARNING [msm-storage-thread-15] de.javakaffee.web.msm.BackupSessionTask.call FAILED for session id AF378F36B5D8310F0CF5B1800B89364B de.javakaffee.web.msm.TranscoderDeserializationException: com.esotericsoftware.kryo.KryoException: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.List java.lang.module.Configuration.parents accessible: module java.base does not "opens java.lang.module" to unnamed module @4eba42b8 Serialization trace: cf (java.lang.ModuleLayer) classLoaderValueMap (jdk.internal.loader.ClassLoaders$AppClassLoader) classloader (java.security.ProtectionDomain) context (java.security.AccessControlContext) acc (java.net.URLClassLoader) classloader (java.security.ProtectionDomain) context (java.security.AccessControlContext) acc (org.apache.catalina.loader.ParallelWebappClassLoader) classLoaderLoggers (org.apache.juli.ClassLoaderLogManager) manager (org.apache.juli.AsyncFileHandler) handlers (java.util.logging.Logger$ConfigurationData) config (java.util.logging.Logger) logger (org.apache.juli.logging.DirectJDKLog) containerLog (org.apache.catalina.valves.AccessLogValve) logs (org.apache.catalina.core.AccessLogAdapter) accessLog (org.apache.catalina.core.StandardHost) container (org.apache.catalina.valves.CrawlerSessionManagerValve) at de.javakaffee.web.msm.serializer.kryo.KryoTranscoder.serializeAttributes(KryoTranscoder.java:240) at de.javakaffee.web.msm.TranscoderService.serializeAttributes(TranscoderService.java:151) at de.javakaffee.web.msm.BackupSessionTask.serializeAttributes(BackupSessionTask.java:179) at de.javakaffee.web.msm.BackupSessionTask.call(BackupSessionTask.java:109) at de.javakaffee.web.msm.BackupSessionTask.call(BackupSessionTask.java:50) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834) Caused by: com.esotericsoftware.kryo.KryoException: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.List java.lang.module.Configuration.parents accessible: module java.base does not "opens java.lang.module" to unnamed module @4eba42b8 Serialization trace: cf (java.lang.ModuleLayer) classLoaderValueMap (jdk.internal.loader.ClassLoaders$AppClassLoader) classloader (java.security.ProtectionDomain) context (java.security.AccessControlContext) acc (java.net.URLClassLoader) classloader (java.security.ProtectionDomain) context (java.security.AccessControlContext) acc (org.apache.catalina.loader.ParallelWebappClassLoader) classLoaderLoggers (org.apache.juli.ClassLoaderLogManager) manager (org.apache.juli.AsyncFileHandler) handlers (java.util.logging.Logger$ConfigurationData) config (java.util.logging.Logger) logger (org.apache.juli.logging.DirectJDKLog) containerLog (org.apache.catalina.valves.AccessLogValve) logs (org.apache.catalina.core.AccessLogAdapter) accessLog (org.apache.catalina.core.StandardHost) container (org.apache.catalina.valves.CrawlerSessionManagerValve) at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:101) at com.esotericsoftware.kryo.serializers.FieldSerializer.write(FieldSerializer.java:508) at com.esotericsoftware.kryo.Kryo.writeClassAndObject(Kryo.java:651) at com.esotericsoftware.kryo.serializers.CollectionSerializer.write(CollectionSerializer.java:100) at com.esotericsoftware.kryo.serializers.CollectionSerializer.write(CollectionSerializer.java:40) at com.esotericsoftware.kryo.Kryo.writeClassAndObject(Kryo.java:651) at com.esotericsoftware.kryo.serializers.MapSerializer.write(MapSerializer.java:113) at com.esotericsoftware.kryo.serializers.MapSerializer.write(MapSerializer.java:39) at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:575) at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:79) at com.esotericsoftware.kryo.serializers.FieldSerializer.write(FieldSerializer.java:508) at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:575) at
[GitHub] [tomcat] kkolinko commented on issue #154: Don't serialize the whole valve into session, but use a smaller object.
kkolinko commented on issue #154: Don't serialize the whole valve into session, but use a smaller object. URL: https://github.com/apache/tomcat/pull/154#issuecomment-480852735 1. Your title is lacking. Your "valve" is CrawlerSessionManagerValve. 2. Behaviour of CrawlerSessionManagerValve versus serialization of sessions is a valid question. At least it needs some comments and documentation improvements. I think this should be filed into Bugzilla and discussed there. (Pull requests come and go, and Bugzilla is what is referenced in changelog). 3. Your pull request does not make any sense to me. The current implementation of CrawlerSessionManagerValve does not implement java.io.Serializable, so it is not written out. Likewise, your CrawlerHttpSessionBindingListener does not implement Serializable. (*) The hashmaps of ids owned by CrawlerSessionManagerValve should exist in 1 instance only (in memory). It does not make sense to serialize them with a session. What am I missing, and what exactly is your problem? (And again, problems would better be filed into Bugzilla). (*) See javadoc for CrawlerSessionManagerValve => "All implemented interfaces": http://tomcat.apache.org/tomcat-9.0-doc/api/org/apache/catalina/valves/CrawlerSessionManagerValve.html This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] mlem opened a new pull request #155: Don't serialize the whole valve into session, but use a smaller object.
mlem opened a new pull request #155: Don't serialize the whole valve into session, but use a smaller object. URL: https://github.com/apache/tomcat/pull/155 It helps to serialize this sessions with libraries like kryo. Background: we've found out, that our kryo library serializes the whole valve. Our stack indicates, that jdk internals are getting serialized into the sessions. It's unnecessary and can be solves more easily with this fix. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] mlem opened a new pull request #156: Don't serialize the whole valve into session, but use a smaller object. 7.0.x
mlem opened a new pull request #156: Don't serialize the whole valve into session, but use a smaller object. 7.0.x URL: https://github.com/apache/tomcat/pull/156 It helps to serialize this sessions with libraries like kryo. Background: we've found out, that our kryo library serializes the whole valve. Our stack indicates, that jdk internals are getting serialized into the sessions. It's unnecessary and can be solves more easily with this fix. (backporting, not an issue with 7.0.x) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] markt-asf commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration.
markt-asf commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration. URL: https://github.com/apache/tomcat/pull/141#issuecomment-468806277 Test ping. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] csutherl commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration.
csutherl commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration. URL: https://github.com/apache/tomcat/pull/141#issuecomment-468824644 Do we want to accept this commit so that the docs mostly reflect our current state and fire off a discussion re: `svn:eol-style`? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] markt-asf commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration.
markt-asf commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration. URL: https://github.com/apache/tomcat/pull/141#issuecomment-468826081 +1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-469229642 Could you please elaborate what you meant by your reference to the "maxIdle" and "initialSize" options ? How do they relate to the "maxAge" one in your opinion ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #140: jdbc-pool: Improve maxAge handling
rmaucher commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-469260446 I think maxAge is ok, but then the pool should indeed have at least minIdle. Basically, if you have 0 active, then you have at least minIdle, that sounds intuitive to me. I'm not doing the pool btw, just commenting based off the thread pool configurations. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #144: Variable adds final modifier
rmaucher commented on issue #144: Variable adds final modifier URL: https://github.com/apache/tomcat/pull/144#issuecomment-470054183 That's a very useful reminder for me, if a PR is pulled, it *also* needs to have a decent description as well, otherwise misleading information gets into the commit info [and for sure it is the sort that would bring us trouble]. So -1 as well for this reason. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 closed pull request #144: Variable adds final modifier
y987425112 closed pull request #144: Variable adds final modifier URL: https://github.com/apache/tomcat/pull/144 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #144: Variable adds final modifier
y987425112 commented on issue #144: Variable adds final modifier URL: https://github.com/apache/tomcat/pull/144#issuecomment-470055949 I am very sorry, this is the first time I submit the code, because I am Chinese, my English is a little poor, there are errors in the description. Thank you very much for your guidance. I will continue to work hard. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #144: Variable adds final modifier
markt-asf commented on issue #144: Variable adds final modifier URL: https://github.com/apache/tomcat/pull/144#issuecomment-470050541 Potential security vulnerabilities should be reported privately to secur...@tomcat.apache.org. Not in a PR or any other public forum. That said, there are no valid security risks here so - fortunately - no harm is done. The additional of final here (and the many, many other places tools such as UCDetector will identify automatically) is more a matter of style than anything else. Making invalid claims of 'security risks' is not helpful. It undermines the credibility of the PR and makes it more likely it will be rejected. We generally do not make changes purely for stylistic reasons. There is a code quality case that could be made for this change but it isn't a particularly strong one. I am -1 on the PR as currently submitted due to the incorrect statement regarding security risks in the commit comment. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #143: Apply deduplication to certain loaded and created Strings
rmaucher commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470022913 Can you give a metric of actual memory gains made using this change ? Adding complexity for very little benefit doesn't seem like a good move overall. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470022349 If there is a lookup with Charset.forName, then would the cache be used at all ? I don't understand this sort of Windows style "startup time" optimization, where you hide stuff behind making running actual application code slower. This is often a bad design, so I don't see what this change brings. -0 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 opened a new pull request #144: Variable adds final modifier
y987425112 opened a new pull request #144: Variable adds final modifier URL: https://github.com/apache/tomcat/pull/144 If these variables do not add final modifiers, there will be ambiguity and security risks. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz edited a comment on issue #143: Apply deduplication to certain loaded and created Strings
ChristopherSchultz edited a comment on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470156059 I like the idea, but I think the implementation could be better. For instance: 1. There is no need to have two collections; just use one and cache everything. 2. The cache should be "weak" so that any value no longer used can be GC'd This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on issue #143: Apply deduplication to certain loaded and created Strings
ChristopherSchultz commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470156059 I like the idea, but I think. the implementation could be better. For instance: 1. There is no need to have two collections; just use one and cache everything. 2. The cache should be "weak" so that any value no longer used can be GC'd This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470145284 Sure. I discovered these by using the "Inspections" feature from [YourKit](https://www.yourkit.com/) on an [embedded Tomcat application](https://github.com/spring-projects/spring-boot/tree/87d696d6971c8bbd28be4fa885f9cf5719309c99/spring-boot-samples/spring-boot-sample-tomcat). Without the change YourKit reports 2192 duplicate Strings, with the change it's 1126 (so a saving of 1066). It's hard to get the exact memory saving but scanning the UI for the top hitters we have: | String | Duplicates | Waste | --- | --- | -- | "java.lang.String" | 2192 | 149112 | "ACTION" | 542 | 38952 | "boolean" | 191 | 10640 | "void" | 204 | 9744 | "Introspected parameter param0" | 80 | 8216 | "int" | 155 | 7392 | "[Ljava.lang.String;" | 42 | 3280 | So it looks like at least 227336 bytes can be saved. One other thing to consider is that I think these Strings are retained for the life of the application. They can't be GC'd because references remain until shutdown. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #143: Apply deduplication to certain loaded and created Strings
rmaucher commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470157903 Ok, I'm not sure it is a very significant saving overall (probably not), but it's still more than I expected. Yes, +1for Chris, it needs to be something like TomcatString.getString I suppose. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470210052 The other option would be to use `String.intern` for these limited cases. Either way, I'm happy to rework the PR if you have a specific direction you'd like me to take it? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on issue #143: Apply deduplication to certain loaded and created Strings
ChristopherSchultz commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470265906 Good point: String.intern is already implemented, and it's already "weak". Less code, same result = better. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #142: Defer creation of encodingToCharsetCache
philwebb commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470317822 I've pushed another update now that I know about https://bz.apache.org/bugzilla/show_bug.cgi?id=51400 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #142: Defer creation of encodingToCharsetCache
philwebb commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470280939 I've pushed an update following the discussion on https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 @rmaucher > If there is a lookup with Charset.forName, then would the cache be used at all Correct. The updated code relies on the [internal cache](https://github.com/openjdk-mirror/jdk7u-jdk/blob/f4d80957e89a19a29bb9f9807d2a28351ed7f7df/src/share/classes/java/nio/charset/Charset.java#L465-L498) that the `Charset` class uses. > I don't understand this sort of Windows style "startup time" optimization, where you hide stuff behind making running actual application code slower I don't know what you mean by "Windows style startup time optimization". Why would the actual actual application code now be slower? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] csutherl merged pull request #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration.
csutherl merged pull request #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration. URL: https://github.com/apache/tomcat/pull/141 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] csutherl commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration.
csutherl commented on issue #141: Update BUILDING and CONTRIBUTING docs after the svn->git migration. URL: https://github.com/apache/tomcat/pull/141#issuecomment-469688472 I've accepted the PR and will follow up on the dev@ list with a discussion on an `svn:eol-style` equivalent. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb opened a new pull request #142: Defer creation of encodingToCharsetCache
philwebb opened a new pull request #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142 Defer creation of `encodingToCharsetCache` until absolutely necessary and update `B2CConverter.getCharset` to first attempt a direct lookup using `Charset.forName`. This change helps improve startup time and reduces the number of classes loaded when `B2CConverter` is only called with standard charsets. See https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb opened a new pull request #143: Apply deduplication to certain loaded and created Strings
philwebb opened a new pull request #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143 Apply deduplication to certain loaded and created Strings by using a static Map. This applies the same technique as described in https://shipilev.net/jvm/anatomy-quarks/10-string-intern/ to reduce the number of String instances that ultimately contain the same contents. See Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] KeiichiFujino commented on issue #140: jdbc-pool: Improve maxAge handling
KeiichiFujino commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-472779046 If testOnConnect is true or initSQL is set, we need validate the connection( with VALIDATE_INIT) when reconnecting. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-472791041 Good catch, I will fix this. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] samslow opened a new pull request #148: Remove duplicated comments
samslow opened a new pull request #148: Remove duplicated comments URL: https://github.com/apache/tomcat/pull/148 Signed-off-by: samslow I don't know why this comments is here. If there is a reason why the comment is here and I do not know, Abort this PR and Teach me if you can This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf merged pull request #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
markt-asf merged pull request #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] nightwatchcyber opened a new pull request #138: Expanding SSI documentation (bug # 63184)
nightwatchcyber opened a new pull request #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138 Fixing: https://bz.apache.org/bugzilla/show_bug.cgi?id=63184 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #138: Expanding SSI documentation (bug # 63184)
markt-asf commented on issue #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138#issuecomment-472856732 Close unintentionally during svn->git migration. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] wilkinsona commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
wilkinsona commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146#issuecomment-472829112 In addition to the improved startup time, this change has an even bigger positive impact on Tomcat's memory footprint. With the latest 9.0 snapshot, I have an app using embedded Tomcat that will start with `-Xmx13M` but not with `-Xmx12M`. The addition of the change proposed here, allows the same app to start with `-Xmx9M`. That ~4MB saving in heap footprint is very welcome indeed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] wilkinsona edited a comment on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
wilkinsona edited a comment on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146#issuecomment-472829112 In addition to the improved startup time, this change has an even bigger positive impact on Tomcat's memory footprint. With the latest 9.0 snapshot, I have an app using embedded Tomcat that will start with `-Xmx13M` but not with `-Xmx12M`. The addition of the change proposed here allows the same app to start with `-Xmx9M`. That ~4MB saving in heap footprint is very welcome indeed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #148: Remove duplicated comments
markt-asf commented on issue #148: Remove duplicated comments URL: https://github.com/apache/tomcat/pull/148#issuecomment-473021367 No reason. It looks like a leftover from the last major redesign. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf merged pull request #148: Remove duplicated comments
markt-asf merged pull request #148: Remove duplicated comments URL: https://github.com/apache/tomcat/pull/148 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-473018240 I fixed the missing calls to PooledConnection#validate(int). While reading the code I was kind of unsure whether I should call PooledConnection#setTimestamp(long) when reconnecting an idle connection. Looking at the code it seems that the timestamp should only be updated when user-initiated (like borrow or return) operations are executed on the PooledConnection, not when (possibly internal) operations are executed on the underlying JDBC Connection - right ? The JavaDoc on setTimestamp() is kind of fuzzy and does not explain what kind of 'operations' are meant. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184)
markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138#discussion_r265735282 ## File path: webapps/docs/ssi-howto.xml ## @@ -146,27 +146,53 @@ false. The directives are: -config - !--#config timefmt=%B %Y -- -Used to set the format of dates and other items processed by SSI +config - !--#config errmsg=Error occured sizefmt=abbrev timefmt=%B %Y -- +Used to set SSI error message, the format of dates and file sizes processed by SSI. +All are optional but at least one must be used. The available options are as follows: + +errmsg - error message used for SSI errors +sizefmt - format used for sizes in the fsize directive +timefmt - format used for timestamps in the flastmod directive -echo - !--#echo var=VARIABLE_NAME -- +echo - !--#echo var=VARIABLE_NAME encoding=entity -- will be replaced by the value of the variable. + +The optional encoding parameter specifies the type of encoding to use. +Valid values are entity (default), url or none. +NOTE: Using an encoding other than entity can lead to security issues. -exec - Used to run commands on the host system. +exec - !--#exec cmd=file-name -- +Used to run commands on the host system. + + +exec - !--#exec cgi=file-name -- +This acts the same as the include directive, and doesn't actually execute any commands. + + +include - !--#include file=file-name -- +inserts the contents, this is interpreted relative to the current directory. include - !--#include virtual=file-name -- -inserts the contents +inserts the contents, this is interpreted relative to the document being served. flastmod - !--#flastmod file=filename.shtml -- -Returns the time that a file was lost modified. +Returns the time that a file was lost modified, this is interpreted relative to the current directory. + + +flastmod - !--#flastmod virtual=filename.shtml -- +Returns the time that a file was lost modified, this is interpreted relative to the document being served. fsize - !--#fsize file=filename.shtml -- -Returns the size of a file. +Returns the size of a file, this is interpreted relative to the current directory. + + +fsize - !--#fsize virtual=filename.shtml -- +Returns the size of a file, this is interpreted relative to the document being served. Review comment: "relative to the document"? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184)
markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138#discussion_r265734162 ## File path: webapps/docs/ssi-howto.xml ## @@ -146,27 +146,53 @@ false. The directives are: -config - !--#config timefmt=%B %Y -- -Used to set the format of dates and other items processed by SSI +config - !--#config errmsg=Error occured sizefmt=abbrev timefmt=%B %Y -- +Used to set SSI error message, the format of dates and file sizes processed by SSI. +All are optional but at least one must be used. The available options are as follows: + +errmsg - error message used for SSI errors +sizefmt - format used for sizes in the fsize directive +timefmt - format used for timestamps in the flastmod directive -echo - !--#echo var=VARIABLE_NAME -- +echo - !--#echo var=VARIABLE_NAME encoding=entity -- will be replaced by the value of the variable. + +The optional encoding parameter specifies the type of encoding to use. +Valid values are entity (default), url or none. +NOTE: Using an encoding other than entity can lead to security issues. -exec - Used to run commands on the host system. +exec - !--#exec cmd=file-name -- +Used to run commands on the host system. + + +exec - !--#exec cgi=file-name -- +This acts the same as the include directive, and doesn't actually execute any commands. + + +include - !--#include file=file-name -- +inserts the contents, this is interpreted relative to the current directory. include - !--#include virtual=file-name -- -inserts the contents +inserts the contents, this is interpreted relative to the document being served. flastmod - !--#flastmod file=filename.shtml -- -Returns the time that a file was lost modified. +Returns the time that a file was lost modified, this is interpreted relative to the current directory. + + +flastmod - !--#flastmod virtual=filename.shtml -- +Returns the time that a file was lost modified, this is interpreted relative to the document being served. Review comment: "last modified" not "lost modified" This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184)
markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138#discussion_r265734995 ## File path: webapps/docs/ssi-howto.xml ## @@ -146,27 +146,53 @@ false. The directives are: -config - !--#config timefmt=%B %Y -- -Used to set the format of dates and other items processed by SSI +config - !--#config errmsg=Error occured sizefmt=abbrev timefmt=%B %Y -- +Used to set SSI error message, the format of dates and file sizes processed by SSI. +All are optional but at least one must be used. The available options are as follows: + +errmsg - error message used for SSI errors +sizefmt - format used for sizes in the fsize directive +timefmt - format used for timestamps in the flastmod directive -echo - !--#echo var=VARIABLE_NAME -- +echo - !--#echo var=VARIABLE_NAME encoding=entity -- will be replaced by the value of the variable. + +The optional encoding parameter specifies the type of encoding to use. +Valid values are entity (default), url or none. +NOTE: Using an encoding other than entity can lead to security issues. -exec - Used to run commands on the host system. +exec - !--#exec cmd=file-name -- +Used to run commands on the host system. + + +exec - !--#exec cgi=file-name -- +This acts the same as the include directive, and doesn't actually execute any commands. + + +include - !--#include file=file-name -- +inserts the contents, this is interpreted relative to the current directory. include - !--#include virtual=file-name -- -inserts the contents +inserts the contents, this is interpreted relative to the document being served. flastmod - !--#flastmod file=filename.shtml -- -Returns the time that a file was lost modified. +Returns the time that a file was lost modified, this is interpreted relative to the current directory. + Review comment: "last" not "lost" "relative to the current directory"? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184)
markt-asf commented on a change in pull request #138: Expanding SSI documentation (bug # 63184) URL: https://github.com/apache/tomcat/pull/138#discussion_r265735188 ## File path: webapps/docs/ssi-howto.xml ## @@ -146,27 +146,53 @@ false. The directives are: -config - !--#config timefmt=%B %Y -- -Used to set the format of dates and other items processed by SSI +config - !--#config errmsg=Error occured sizefmt=abbrev timefmt=%B %Y -- +Used to set SSI error message, the format of dates and file sizes processed by SSI. +All are optional but at least one must be used. The available options are as follows: + +errmsg - error message used for SSI errors +sizefmt - format used for sizes in the fsize directive +timefmt - format used for timestamps in the flastmod directive -echo - !--#echo var=VARIABLE_NAME -- +echo - !--#echo var=VARIABLE_NAME encoding=entity -- will be replaced by the value of the variable. + +The optional encoding parameter specifies the type of encoding to use. +Valid values are entity (default), url or none. +NOTE: Using an encoding other than entity can lead to security issues. -exec - Used to run commands on the host system. +exec - !--#exec cmd=file-name -- +Used to run commands on the host system. + + +exec - !--#exec cgi=file-name -- +This acts the same as the include directive, and doesn't actually execute any commands. + + +include - !--#include file=file-name -- +inserts the contents, this is interpreted relative to the current directory. include - !--#include virtual=file-name -- -inserts the contents +inserts the contents, this is interpreted relative to the document being served. flastmod - !--#flastmod file=filename.shtml -- -Returns the time that a file was lost modified. +Returns the time that a file was lost modified, this is interpreted relative to the current directory. + + +flastmod - !--#flastmod virtual=filename.shtml -- +Returns the time that a file was lost modified, this is interpreted relative to the document being served. fsize - !--#fsize file=filename.shtml -- -Returns the size of a file. +Returns the size of a file, this is interpreted relative to the current directory. + Review comment: "relative to the current directory"? That looks like a copy/paste error. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] KeiichiFujino commented on issue #140: jdbc-pool: Improve maxAge handling
KeiichiFujino commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-473776997 Thanks for the fix. I have no objections to the current code. I'll merge this into master branch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling
toby1984 commented on issue #140: jdbc-pool: Improve maxAge handling URL: https://github.com/apache/tomcat/pull/140#issuecomment-471474038 Sorry for not getting back any earlier. I did not touch the existing code responsible for discarding idle connections or the creation of new connections, so if your statement "if you have 0 active, then you have at least minIdle" used to be true in the past, it still holds now. The only change to the previous behaviour that *might* be perceived by a user is that busy connections returned to the pool that also exceeded maxAge are not getting discarded anymore but instead only the underlying JDBC connection gets recreated. This is only visible to a user if the pool is not being used again right away, otherwise a new connection would've been created and the change would be imperceivable. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488862
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
```
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
Dead cycle
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470489493
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
```
Dead cycle
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #142: Defer creation of encodingToCharsetCache
philwebb commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470546205 > Besides the 31ms startup time, it's still worse in every way compared to the current code. I'm really not sure what I'm supposed to do with a comment like that. As someone who's just trying to help contribute a fix for an identified problem, it really doesn't make me feel very welcome :( I'm happy to have another go at fixing this if you can provide something more constructive than "it's worse in every way". Having the current code call `availableCharsets()` which is specifically documented with "may cause time-consuming disk or network I/O operations to occur" in a static initializer with no way to opt-out doesn't seem ideal either. AFAICT this is something that neither Jetty or Undertow do, so I don't believe it's a problem that _can't_ be solved in another way. I understand that Tomcat has many users that start it only once, and aren't really bothered by these kind of optimizations. To them 30ms and not loading an extra 50-60 classes isn't really a bit deal. However, I strongly believe that there is another class of user that want to run Embedded Tomcat and will care deeply about this kind of thing. If the type of change I'm suggesting is never going to be acceptable, perhaps I can change my approach. Would it be better if I tried to make the charset lookup code plugabble? That way, the existing code would not need to change, but users who do care about this issue could replace the lookup code with something else. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470503719
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
If no instruction reordering occurs, this line of code will never run
`System.out.println("Wrong result");`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470503719
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
If no instruction reordering occurs, this line of code will never run
`System.out.println("Wrong result");`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #145: Adding volatile keywords to member variables
markt-asf closed pull request #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #145: Adding volatile keywords to member variables
markt-asf commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470502673 The claims made above are incorrect. They fail to take account of the requirement that > However, compilers are allowed to reorder the instructions in each thread, when this **does not affect the execution of that thread in isolation**. (my emphasis) The claimed re-ordering is not possible because it changes the behaviour of the code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rainerjung commented on issue #145: Adding volatile keywords to member variables
rainerjung commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470501897 Yes of course, because the threads do not oby to the rules I have described. From my comment: "Changes done by thread T1 while holding any lock are visible by any other thread T2 provided that T1 releases the lock after the changes and T2 acquired it before accessing the changed data." This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470504073
```
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
```
// If no instruction reordering occurs, this line of code will never run
System.out.println("Wrong result");
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 opened a new pull request #145: Adding volatile keywords to member variables
y987425112 opened a new pull request #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145
The semantics of the Java programming language allow compilers and
microprocessors to
perform optimizations that can interact with incorrectly synchronized code
in ways that can
produce behaviors that seem
paradoxical.[https://edelivery.oracle.com/otn-pub/jcp/memory_model-1.0-pfd-spec-oth-JSpec/memory_model-1_0-pfd-spec.pdf](url)
page6
For example
```
org.apache.catalina.core.StandardContext.java
private String applicationListeners[] = new String[0];
.
@Override
public void addApplicationListener(String listener) {
```
synchronized (applicationListenersLock) {
String results[] = new String[applicationListeners.length + 1];
for (int i = 0; i < applicationListeners.length; i++) {
if (listener.equals(applicationListeners[i])) {
log.info(sm.getString("standardContext.duplicateListener",listener));
return;
}
results[i] = applicationListeners[i];
}
results[applicationListeners.length] = listener;
applicationListeners = results;
}
fireContainerEvent("addApplicationListener", listener);
}
We expect this line of code(`applicationListeners = results;`) to be
executed eventually
But because of instruction reordering,The following operating procedures are
also allowed to exis
```
@Override
public void addApplicationListener(String listener) {
```
synchronized (applicationListenersLock) {
String results[] = new String[applicationListeners.length + 1];
applicationListeners = results;
for (int i = 0; i < applicationListeners.length; i++) {
if (listener.equals(applicationListeners[i])) {
log.info(sm.getString("standardContext.duplicateListener",listener));
return;
}
results[i] = applicationListeners[i];
}
results[applicationListeners.length] = listener;
}
fireContainerEvent("addApplicationListener", listener);
}
If this happens, it will be different from our expectations.
If volatile modifiers are added to variables, instruction reordering is
prohibited
`private String applicationListeners[] = new String[0]; `-->
` private volatile String applicationListeners[] = new String[0];`
Satisfy the purpose of the last execution of this line of
code(`applicationListeners = results;`).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470434736 Besides the 31ms startup time, it's still worse in every way compared to the current code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488862
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
```
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
Dead cycle
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488340
`package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}`
Dead cycle
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rainerjung commented on issue #145: Adding volatile keywords to member variables
rainerjung commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470482121 My understanding always was, that reordering must obey as-if-serial semantics for any single thread. The complexity only kicks in when checking, whether effects of statements run by one thread are visible by another. The concrete example you discuss IMHO does not have any visibility problems, because the changes are inside a synchronized block. Changes done by thread T1 while holding any lock are visible by any other thread T2 provided that T1 releases the lock after the changes and T2 acquired it before accessing the changed data. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488340
`package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}`
Dead cycle
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #142: Defer creation of encodingToCharsetCache
markt-asf commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470607054 No. Not an system property ;). Seriously, I think there is a way that we can improve start-up time without impacting the vast majority of users. Phil's patch is heading in the general direction I was thinking. As ever, there are likely to be trade-offs involved. I hope to be able to have some hard numbers on which to base that discussion soon(ish). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470602748 You submitted three items, and indeed I am not convinced by this one. Looking back to the old BZs, there was also there the conclusion that the current code is the best solution for the general Tomcat use case. However, there is the "but I only want UTF-8" argument, in which case the solution is to hardcode UTF-8 and default to Charset.forName for the rest. I think system property configuration is supposed to be removed, but it would be a good solution for this pluggability (it's global, no one really actually needs it, etc). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf opened a new pull request #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
markt-asf opened a new pull request #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146 Implement an alternative Charset caching strategy that does not load all Charsets at Tomcat start. This reduces start time by ~30ms and does not, based on the performance tests included in this commit, have a negative impact on runtime look-ups. It does require that the names of all supported Charsets are known to Tomcat at compile time. The code has been tested with a range of JVMs and a unit test is provided for testing new JVMs. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #142: Defer creation of encodingToCharsetCache
markt-asf commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-471072264 See #146 for an alternative 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
rmaucher commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146#issuecomment-471178688 Ok I guess. Wasn't the plan going to include hardcoding UTF-8 ? The previous code still looks better to me (HashMap vs concurrent HashMap) but as you seem to care about resolving this now this is ok with me. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
markt-asf commented on issue #146: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63235 URL: https://github.com/apache/tomcat/pull/146#issuecomment-471181388 Thanks for taking a look at the code. I was concerned about the HashMap vs ConcurrentHashMap as well but in the tests included with the patch the difference was very small. Averaged over several runs it looked like ConcurrentHashMap was slower but by less than 1ns per lookup on average. That seemed small enough not to worry about to me. In terms of special handling of UTF-8 my plan was only to pre-load the cache for UTF-8 and ISO-8859-1. I thought about adding other charsets but given ISO-8859-1 is the spec default and nearly everyone uses UTF-8 those seemed like the best place to start. Anything else I could think of just added more complexity and reduced performance. I'm currently leaning towards applying this after the next 9.0.x tag so there is still plenty of time for folks to review and suggest improvements. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470381178 I've pushed an update that uses `String.intern` for those two places. With this change 2192 duplicates are reduced to 300 and I'm not seeing any difference in startup time. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmannibucau commented on issue #143: Apply deduplication to certain loaded and created Strings
rmannibucau commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470392703 Is generating a java class at build time instead of reading the xml possible? Then a preregistration in a registry would enable to bypass a lot of xml parsing and reflection which should give a real boost. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470667817 @rmannibucau I've opened another issue for that https://bz.apache.org/bugzilla/show_bug.cgi?id=63237. If I get a second I'll try some experiments. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] salgattas opened a new pull request #149: Adding ReDoS warning/documentation to RewriteValve
salgattas opened a new pull request #149: Adding ReDoS warning/documentation to RewriteValve URL: https://github.com/apache/tomcat/pull/149 After reporting a potential DoS in "Rewrite Rules" to the Tomcat security team, it was decided that there was no bug in Tomcat itself, but rather in how a user sets up their Tomcat server. Thus, I was instructed by the security team to create a PR for updated documentation to better educate users on appropriate usage of Rewrite Rules. This commit added javadoc comments for the RewriteValve class, as instructed. Furthermore, I'd like to update the documentation on this page as well, however I cannot find a mechanism to do so: https://tomcat.apache.org/tomcat-9.0-doc/rewrite.html This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10
tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-474872724 Ok sorry I didn't understand the first comment So I will update the PR like you said This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10
markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-474861978 What I meant was: - we should check for ECJ updates (I'm doing that now) - we should replicate the way Java 10 support is handled in Tomcat 7 in all Tomcat versions when ECJ doesn't have the necessary constants for the newer Java versions. If you wanted to update your PR along those lines that would be great. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10
tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-474878836 Ok I have add 11 and 12, I wait for your commit, and I will update this with just 12 if necessary thx This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10
markt-asf commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-474873975 I actually just done this for Java 11. Could you do the same for Java 12? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] tcollignon opened a new pull request #150: Add compiler source and target support for all Java versions over 10
tcollignon opened a new pull request #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150 Hi, I maintain the maven plugin to compile JSP : https://github.com/leonardehrenfried/jspc-maven-plugin We have receive an issue when someone trying to compile JSP with Java 11 (to obtain target jsp class in Java 11 bytecode) see https://github.com/leonardehrenfried/jspc-maven-plugin/issues/39 I have look at the JDTCompiler source code and it appear that it handle only Java 10. So I propose this modifications : - If compiler source or target is set, and value is not known by ECJ constant, we even set this directly and not fallback to Java 1.8. Java move faster now, and probably ECJ will not move as faster. - If compiler source or target is not set, we fallback to Java 1.8 as previous WDYT ? Thanks you all This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10
tcollignon commented on issue #150: Add compiler source and target support for all Java versions over 10 URL: https://github.com/apache/tomcat/pull/150#issuecomment-474831606 Thx for your anwser So I will wait for ECJ right ? Or maybe I can try to add options to Java 11 in ECJ ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
