Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

tomcat_logging_support isn't compatible with java9 #530

Closed
youngm opened this issue Dec 12, 2017 · 7 comments
Closed

tomcat_logging_support isn't compatible with java9 #530

youngm opened this issue Dec 12, 2017 · 7 comments
Assignees
Milestone

Comments

@youngm
Copy link
Contributor

youngm commented Dec 12, 2017

Java9 removed support for the -Djava.endorsed.dirs system property. When starting with this command I get the following error in my logs:

   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] OUT JVM Memory Configuration: -XX:MaxDirectMemorySize=10M -XX:MaxMetaspaceSize=21522K -XX:ReservedCodeCacheSize=240M -Xmx694253K
   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] OUT -Djava.endorsed.dirs=/home/vcap/app/.java-buildpack/tomcat/endorsed is not supported. Endorsed standards and standalone APIs
   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] OUT in modular form will be supported via the concept of upgradeable modules.
   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] ERR Error: A fatal exception has occurred. Program will exit.
   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] ERR NOTE: Picked up JDK_JAVA_OPTIONS:  --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.rmi/sun.rmi.transport=ALL-UNNAMED
   2017-12-11T17:36:04.50-0700 [APP/PROC/WEB/0] ERR Error: Could not create the Java Virtual Machine.
   2017-12-11T17:36:04.51-0700 [APP/PROC/WEB/0] OUT Exit status 1

It appears Tomcat 8.5.24 made it so that if the "endorsed" directory doesn't exist it won't attempt to add that argument. However, the java buildpack adds that directory to support tomcat_logging_support.

It appears we'll need to provide a different mechanism to support this feature in java9+Tomcat.

@youngm
Copy link
Contributor Author

youngm commented Dec 12, 2017

Link to Tomcat release notes with change. Under "Improve handling of endorsed directories": http://tomcat.apache.org/tomcat-8.5-doc/changelog.html#Tomcat_8.5.24_(markt)

@nebhale
Copy link
Member

nebhale commented Dec 12, 2017

Yep, on my mind when I said "nothing else is ready to go". I have no idea what the work around is for something endorsed in Tomcat though. Maybe I'll have a chat with @markt-asf.

@nebhale nebhale self-assigned this Dec 12, 2017
@nebhale nebhale added this to the v4.8 milestone Dec 12, 2017
@mayrstefan
Copy link
Contributor

Was there a special use case for the endorsed functionality?

If not, I see three options:

  1. put jars into tomcat/lib
  2. make use of different CATALINA_HOME and CATALINA_BASE directories and put jars into ${catalina.base}/lib
  3. put jars into a separate "jbp" directory and include it in the conf/catalina.properties entry for common.loader

@youngm
Copy link
Contributor Author

youngm commented Dec 22, 2017

@mayrstefan Here is the original issue that added this functionality: #30 In my past experience ${catalina.base}/lib wasn't early enough in the classloader for logging.properties to pick up custom logging config but perhaps that has changed over the years?

@mayrstefan
Copy link
Contributor

I did some quick and dirty tests. I had some offline buildpacks on my machine that included different Tomcat versions:

  • 7.0.70
  • 8.0.38
  • 8.5.23

In those zip-files I patched lib/java_buildpack/container/tomcat/tomcat_logging_support.rb to be

      def compile
        download_jar(jar_name, lib)
      end

      def lib
        @droplet.sandbox + 'lib'
      end

      def release
      end

Then I deployed a small servlet that did a System.out.println and System.err.println. Both lines were streamed to cf logs. Those tests were done using Java 8 (I only wanted to get rid of the endorsed directory). That seemed to work so far. Is there a test application to verify all logging cases?

@nebhale
Copy link
Member

nebhale commented Dec 24, 2017

It is not. The logging is Tomcat's own container logging. This is why it is up so high in endorsed, so that Tomcat will load those JARs in it’s root classloader. The test is to use our configuration while putting the JARs into alternate locations and seeing all the results stream out to stdout.

@mayrstefan
Copy link
Contributor

Okay. I had a look into the Tomcat catalina.sh script. It generates a classpath string with the tomcat-juli.jar appended to it. To extend the CLASSPATH variable we can use setenv.sh. More copy & paste resulted into this patch:

--- lib/java_buildpack/container/tomcat/tomcat_logging_support.rb.orig	2017-10-17 14:12:16.000000000 +0200
+++ lib/java_buildpack/container/tomcat/tomcat_logging_support.rb	2017-12-24 15:04:48.710981716 +0100
@@ -14,23 +14,25 @@
 # limitations under the License.
 
 require 'java_buildpack/component/versioned_dependency_component'
+require 'java_buildpack/container/tomcat/tomcat_utils'
+require 'java_buildpack/container'
 
 module JavaBuildpack
   module Container
 
     # Encapsulates the detect, compile, and release functionality for Tomcat logging support.
     class TomcatLoggingSupport < JavaBuildpack::Component::VersionedDependencyComponent
+      include JavaBuildpack::Container
 
       # (see JavaBuildpack::Component::BaseComponent#compile)
       def compile
-        download_jar(jar_name, endorsed)
+        download_jar(jar_name, tomcat_lib)
+        env = "CLASSPATH=$PWD/#{tomcat_lib.relative_path_from(@droplet.root)}/#{jar_name}"
+        write_env setenv_sh, env
       end
 
       # (see JavaBuildpack::Component::BaseComponent#release)
-      def release
-        @droplet.java_opts.add_system_property 'java.endorsed.dirs',
-                                               "$PWD/#{endorsed.relative_path_from(@droplet.root)}"
-      end
+      def release; end
 
       protected
 
@@ -41,8 +43,12 @@
 
       private
 
-      def endorsed
-        @droplet.sandbox + 'endorsed'
+      def setenv_sh
+        @droplet.sandbox + 'bin/setenv.sh'
+      end
+
+      def write_env(file, document)
+        file.open('w') { |f| f.write document }
       end
 
       def jar_name

I'm not sure if everything I did is really needed or correct. But my modified 4.6 offline buildpack now runs without an endorsed directory and my outout from java.uitl.logging.Logger starts with [CONTAINER]

@nebhale nebhale modified the milestones: v4.8, v4.9 Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants