On Tue, Jul 21, 2020 at 11:16 PM <fha...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> fhanik pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>      new 098c4c8  Simpler way to determine Graal runtime
> 098c4c8 is described below
>
> commit 098c4c81602ba1e8d5f33b0795d7caf55f70d573
> Author: Filip Hanik <fha...@pivotal.io>
> AuthorDate: Tue Jul 21 14:04:57 2020 -0700
>
>     Simpler way to determine Graal runtime
>
>     Also, allows to mock the behavior
>
> https://www.graalvm.org/sdk/javadoc/org/graalvm/nativeimage/ImageInfo.html#PROPERTY_IMAGE_CODE_KEY
> ---
>  java/org/apache/jasper/runtime/JspRuntimeLibrary.java | 16
> +---------------
>  java/org/apache/naming/NamingContext.java             | 15 +--------------
>  java/org/apache/tomcat/util/compat/GraalCompat.java   | 15 +--------------
>  3 files changed, 3 insertions(+), 43 deletions(-)
>
> diff --git a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
> b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
> index cfb6e75..f27ce3b 100644
> --- a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
> +++ b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
> @@ -56,21 +56,7 @@ import org.apache.tomcat.InstanceManager;
>   */
>  public class JspRuntimeLibrary {
>
> -    public static final boolean GRAAL;
> -
> -    static {
> -        boolean result = false;
> -        try {
> -            Class<?> nativeImageClazz =
> Class.forName("org.graalvm.nativeimage.ImageInfo");
> -            result =
> nativeImageClazz.getMethod("inImageCode").invoke(null) != null;
> -            // Note: This will also be true for the Graal substrate VM
>

As the comment says, this must also be true when running on the substrate
VM (= building a native image) and from what I can see this is not the
case. Basically the code paths used on Graal must be consistent.
So it's simpler but it doesn't seem to work at this time, so you need to
revert this commit. You could get out of this by saying the user can just
set the system property, but this makes things more error prone so it's a
bad idea as the previous solution worked just fine.

I see an enhancement to fix this as the agent would set the system
property: https://github.com/oracle/graal/issues/2395
But the Oracle folks said "no" because they can. As usual :D

Rémy


> -        } catch (ClassNotFoundException e) {
> -            // Must be Graal
> -        } catch (ReflectiveOperationException | IllegalArgumentException
> e) {
> -            // Should never happen
> -        }
> -        GRAAL = result;
> -    }
> +    public static final boolean GRAAL =
> System.getProperty("org.graalvm.nativeimage.imagecode") != null;
>
>      /**
>       * Returns the value of the jakarta.servlet.error.exception request
> diff --git a/java/org/apache/naming/NamingContext.java
> b/java/org/apache/naming/NamingContext.java
> index 0471279..40f4085 100644
> --- a/java/org/apache/naming/NamingContext.java
> +++ b/java/org/apache/naming/NamingContext.java
> @@ -792,20 +792,7 @@ public class NamingContext implements Context {
>      // ------------------------------------------------------ Protected
> Methods
>
>
> -    private static final boolean GRAAL;
> -
> -    static {
> -        boolean result = false;
> -        try {
> -            Class<?> nativeImageClazz =
> Class.forName("org.graalvm.nativeimage.ImageInfo");
> -            result =
> Boolean.TRUE.equals(nativeImageClazz.getMethod("inImageCode").invoke(null));
> -        } catch (ClassNotFoundException e) {
> -            // Must be Graal
> -        } catch (ReflectiveOperationException | IllegalArgumentException
> e) {
> -            // Should never happen
> -        }
> -        GRAAL = result;
> -    }
> +    private static final boolean GRAAL =
> System.getProperty("org.graalvm.nativeimage.imagecode") != null;
>
>      /**
>       * Retrieves the named object.
> diff --git a/java/org/apache/tomcat/util/compat/GraalCompat.java
> b/java/org/apache/tomcat/util/compat/GraalCompat.java
> index 9fb835a..e3cb09d 100644
> --- a/java/org/apache/tomcat/util/compat/GraalCompat.java
> +++ b/java/org/apache/tomcat/util/compat/GraalCompat.java
> @@ -20,20 +20,7 @@ import java.io.IOException;
>
>  class GraalCompat extends Jre9Compat {
>
> -    private static final boolean GRAAL;
> -
> -    static {
> -        boolean result = false;
> -        try {
> -            Class<?> nativeImageClazz =
> Class.forName("org.graalvm.nativeimage.ImageInfo");
> -            result =
> Boolean.TRUE.equals(nativeImageClazz.getMethod("inImageCode").invoke(null));
> -        } catch (ClassNotFoundException e) {
> -            // Must be Graal
> -        } catch (ReflectiveOperationException | IllegalArgumentException
> e) {
> -            // Should never happen
> -        }
> -        GRAAL = result;
> -    }
> +    private static final boolean GRAAL =
> System.getProperty("org.graalvm.nativeimage.imagecode") != null;
>
>      static boolean isSupported() {
>          // This property does not exist for a native image
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Reply via email to