logging-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nickwilli...@apache.org
Subject svn commit: r1519241 - in /logging/log4j/log4j2/trunk: log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ log4j-core/src/test/java/org/apache/logging/log4j/ log4j-core/src/test/...
Date Sun, 01 Sep 2013 03:50:21 GMT
Author: nickwilliams
Date: Sun Sep  1 03:50:20 2013
New Revision: 1519241

URL: http://svn.apache.org/r1519241
Log:
[LOG4J2-322] Centralized reflective use of Reflection#getCallerClass and properly handled
its instability in various versions of Java. In the process, generally refactored ThrowableProxy
and ClassLoaderContextSelector to be somewhat cleaner. Hopefully more work can be done on
this once Java 8's getCallerClass future is certain (in a good way).

Added:
    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReflectiveCallerClassUtility.java
    logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ReflectionComparison.java
      - copied, changed from r1519188, logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/ReflectionComparison.java
Removed:
    logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/ReflectionComparison.java
Modified:
    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
    logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java
    logging/log4j/log4j2/trunk/src/changes/changes.xml

Added: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReflectiveCallerClassUtility.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReflectiveCallerClassUtility.java?rev=1519241&view=auto
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReflectiveCallerClassUtility.java
(added)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ReflectiveCallerClassUtility.java
Sun Sep  1 03:50:20 2013
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.log4j.core.impl;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.helpers.Loader;
+import org.apache.logging.log4j.status.StatusLogger;
+
+/**
+ * Utility class that handles the instability of the Sun/OpenJDK {@code sun.reflect.Reflection.getCallerClass(int)}
+ * method.<br>
+ * <br>
+ * <strong>Background:</strong> This method, available only in the Oracle/Sun/OpenJDK
implementations of the Java
+ * Virtual Machine, is a much more efficient mechanism for determining the {@link Class}
of the caller of a particular
+ * method. When it is not available, a {@link SecurityManager} is the second-best option.
When this is also not
+ * possible, the {@code StackTraceElement[]} returned by {@link Thread#getStackTrace()} must
be used, and its
+ * {@code String} class name converted to a {@code Class} using the slow {@link Class#forName}.<br>
+ * <br>
+ * As of Java 8, the {@code getCallerClass(int)} method has been removed from Oracle/OpenJDK
and is no longer usable. A
+ * back-port of the feature that resulted in this change was made in Java 7u25, but the {@code
getCallerClass(int)} was
+ * left around for that version and deprecated, with the intention of being removed in 7u40.
By coincidence, the change
+ * actually broke {@code getCallerClass(int)} (the return value was inadvertently offset
by 1 stack frame). This was
+ * actually a good thing, because it made the hundreds of libraries and frameworks relying
on this method aware of what
+ * the JDK developers were up to.<br>
+ * <br>
+ * After much community backlash, the JDK team agreed to restore {@code getCallerClass(int)}
and keep its existing
+ * behavior for the rest of Java 7. However, the method will still not be available in Java
8, and so backup options
+ * must be used. This class:<br>
+ * <ul>
+ *     <li>Uses {@code getCallerClass(int)} the traditional way when possible.</li>
+ *     <li>Uses {@code getCallerClass(int)} with an adjusted offset in Oracle/OpenJDK
7u25.</li>
+ *     <li>Returns null otherwise. (Currently, it is the caller's responsibility to
use the backup mechanisms.)</li>
+ * </ul>
+ * <br>
+ * <strong>IMPORTANT NOTE:</strong> This class should not be relied upon. It
is considered an internal class and could
+ * change at any time, breaking your code if you use it. Specifically, as a possible public
API replacement for
+ * {@code getCallerClass(int)} develops in Java 8, this class is very likely to change or
even go away.
+ */
+public final class ReflectiveCallerClassUtility {
+
+    private static final Logger LOGGER = StatusLogger.getLogger();
+
+    private static final boolean GET_CALLER_CLASS_SUPPORTED;
+
+    private static final Method GET_CALLER_CLASS_METHOD;
+
+    static final int JAVA_7U25_COMPENSATION_OFFSET;
+
+    static {
+        Method getCallerClass = null;
+        int java7u25CompensationOffset = 0;
+
+        try {
+            final ClassLoader loader = Loader.getClassLoader();
+            // Use wildcard to avoid compile-time reference.
+            final Class<?> clazz = loader.loadClass("sun.reflect.Reflection");
+            final Method[] methods = clazz.getMethods();
+            for (final Method method : methods) {
+                final int modifier = method.getModifiers();
+                final Class<?>[] parameterTypes = method.getParameterTypes();
+                if (method.getName().equals("getCallerClass") && Modifier.isStatic(modifier)
&&
+                        parameterTypes.length == 1 && parameterTypes[0] == int.class)
{
+                    getCallerClass = method;
+                    break;
+                }
+            }
+
+            if (getCallerClass == null) {
+                LOGGER.info("sun.reflect.Reflection#getCallerClass does not exist.");
+            } else {
+                Object o = getCallerClass.invoke(null, 0);
+                if (o == null || o != clazz) {
+                    getCallerClass = null;
+                    LOGGER.warn("sun.reflect.Reflection#getCallerClass returned unexpected
value of [{}] and is " +
+                            "unusable. Will fall back to another option.", o);
+                } else {
+                    o = getCallerClass.invoke(null, 1);
+                    if (o == clazz) {
+                        java7u25CompensationOffset = 1;
+                        LOGGER.warn("sun.reflect.Reflection#getCallerClass is broken in Java
7u25. " +
+                                "You should upgrade to 7u40. Using alternate stack offset
to compensate.");
+                    }
+                }
+            }
+        } catch (final ClassNotFoundException e) {
+            LOGGER.info("sun.reflect.Reflection is not installed.");
+        } catch (final IllegalAccessException e) {
+            LOGGER.info("sun.reflect.Reflection#getCallerClass is not accessible.");
+        } catch (final InvocationTargetException e) {
+            LOGGER.info("sun.reflect.Reflection#getCallerClass is not supported.");
+        }
+
+        if (getCallerClass == null) {
+            GET_CALLER_CLASS_SUPPORTED = false;
+            GET_CALLER_CLASS_METHOD = null;
+            JAVA_7U25_COMPENSATION_OFFSET = -1;
+        } else {
+            GET_CALLER_CLASS_SUPPORTED = true;
+            GET_CALLER_CLASS_METHOD = getCallerClass;
+            JAVA_7U25_COMPENSATION_OFFSET = java7u25CompensationOffset;
+        }
+    }
+
+    private ReflectiveCallerClassUtility() {
+
+    }
+
+    /**
+     * Indicates whether {@code getCallerClass(int)} can be used on this JVM.
+     *
+     * @return {@code true} if it can be used. If {@code false}, {@link #getCaller} should
not be called. Use a backup
+     *         mechanism instead.
+     */
+    public static boolean isSupported() {
+        return GET_CALLER_CLASS_SUPPORTED;
+    }
+
+    /**
+     * Reflectively calls {@code getCallerClass(int)}, compensating for the additional frame
on the stack, and
+     * compensating for the Java 7u25 bug if necessary. You should check with {@link #isSupported}
before using this
+     * method.
+     *
+     * @param depth The depth of the caller to retrieve.
+     * @return the caller class, or {@code null} if {@code getCallerClass(int)} is not supported.
+     */
+    public static Class<?> getCaller(int depth) {
+        if (!GET_CALLER_CLASS_SUPPORTED) {
+            return null;
+        }
+
+        try {
+            return (Class<?>) GET_CALLER_CLASS_METHOD.invoke(null, depth + 1 + JAVA_7U25_COMPENSATION_OFFSET);
+        } catch (IllegalAccessException ignore) {
+            LOGGER.warn("Should not have failed to call getCallerClass.");
+        } catch (InvocationTargetException ignore) {
+            LOGGER.warn("Should not have failed to call getCallerClass.");
+        }
+        return null;
+    }
+}

Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java?rev=1519241&r1=1519240&r2=1519241&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
(original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
Sun Sep  1 03:50:20 2013
@@ -18,7 +18,6 @@ package org.apache.logging.log4j.core.im
 
 import java.io.Serializable;
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.security.CodeSource;
 import java.util.HashMap;
@@ -27,7 +26,6 @@ import java.util.Map;
 import java.util.Stack;
 
 import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.core.helpers.Loader;
 import org.apache.logging.log4j.status.StatusLogger;
 
 /**
@@ -37,17 +35,15 @@ public class ThrowableProxy implements S
 
     private static final long serialVersionUID = -2752771578252251910L;
 
-    private static Method getCallerClass;
+    private static final Logger LOGGER = StatusLogger.getLogger();
 
-    private static PrivateSecurityManager securityManager;
+    private static final PrivateSecurityManager SECURITY_MANAGER;
 
-    private static final Logger LOGGER = StatusLogger.getLogger();
+    private static final Method GET_SUPPRESSED;
 
-    private static Method getSuppressed;
-    private static Method addSuppressed;
+    private static final Method ADD_SUPPRESSED;
 
     private final ThrowableProxy proxyCause;
-    private int commonElementCount;
 
     private final Throwable throwable;
 
@@ -55,10 +51,38 @@ public class ThrowableProxy implements S
 
     private final StackTracePackageElement[] callerPackageData;
 
+    private int commonElementCount;
 
     static {
-        setupCallerCheck();
-        versionCheck();
+        if (ReflectiveCallerClassUtility.isSupported()) {
+            SECURITY_MANAGER = null;
+        } else {
+            PrivateSecurityManager securityManager;
+            try {
+                securityManager = new PrivateSecurityManager();
+                if (securityManager.getClasses() == null) {
+                    // This shouldn't happen.
+                    securityManager = null;
+                    LOGGER.error("Unable to obtain call stack from security manager.");
+                }
+            } catch (final Exception e) {
+                securityManager = null;
+                LOGGER.debug("Unable to install security manager.", e);
+            }
+            SECURITY_MANAGER = securityManager;
+        }
+
+        Method getSuppressed = null, addSuppressed = null;
+        final Method[] methods = Throwable.class.getMethods();
+        for (final Method method : methods) {
+            if (method.getName().equals("getSuppressed")) {
+                getSuppressed = method;
+            } else if (method.getName().equals("addSuppressed")) {
+                addSuppressed = method;
+            }
+        }
+        GET_SUPPRESSED = getSuppressed;
+        ADD_SUPPRESSED = addSuppressed;
     }
 
     /**
@@ -173,6 +197,7 @@ public class ThrowableProxy implements S
      * @param cause The Throwable to format.
      * @param packages The List of packages to be suppressed from the trace.
      */
+    @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
     public void formatWrapper(final StringBuilder sb, final ThrowableProxy cause, final List<String>
packages) {
         final Throwable caused = cause.getCause() != null ? cause.getCause().getThrowable()
: null;
         if (caused != null) {
@@ -227,6 +252,7 @@ public class ThrowableProxy implements S
         return sb.toString();
     }
 
+    @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
     private void formatCause(final StringBuilder sb, final ThrowableProxy cause, final List<String>
packages) {
         sb.append("Caused by: ").append(cause).append("\n");
         formatElements(sb, cause.commonElementCount, cause.getThrowable().getStackTrace(),
cause.callerPackageData,
@@ -298,17 +324,17 @@ public class ThrowableProxy implements S
      * @return A Deque containing the current stack of Class objects.
      */
     private Stack<Class<?>> getCurrentStack() {
-        if (getCallerClass != null) {
+        if (ReflectiveCallerClassUtility.isSupported()) {
             final Stack<Class<?>> classes = new Stack<Class<?>>();
-            int index = 2;
-            Class<?> clazz = getCallerClass(index);
+            int index = 1;
+            Class<?> clazz = ReflectiveCallerClassUtility.getCaller(index);
             while (clazz != null) {
                 classes.push(clazz);
-                clazz = getCallerClass(++index);
+                clazz = ReflectiveCallerClassUtility.getCaller(++index);
             }
             return classes;
-        } else if (securityManager != null) {
-            final Class<?>[] array = securityManager.getClasses();
+        } else if (SECURITY_MANAGER != null) {
+            final Class<?>[] array = SECURITY_MANAGER.getClasses();
             final Stack<Class<?>> classes = new Stack<Class<?>>();
             for (final Class<?> clazz : array) {
                 classes.push(clazz);
@@ -421,25 +447,6 @@ public class ThrowableProxy implements S
     }
 
     /**
-     * Invoke Reflection.getCallerClass via reflection. This is slightly slower than calling
the method
-     * directly but removes any dependency on Sun's JDK being present at compile time. The
difference
-     * can be measured by running the ReflectionComparison test.
-     * @param index The index into the stack trace.
-     * @return The Class at the specified stack entry.
-     */
-    private Class<?> getCallerClass(final int index) {
-        if (getCallerClass != null) {
-            try {
-                final Object[] params = new Object[]{index};
-                return (Class<?>) getCallerClass.invoke(null, params);
-            } catch (final Exception ex) {
-                // logger.debug("Unable to determine caller class via Sun Reflection", ex);
-            }
-        }
-        return null;
-    }
-
-    /**
      * Loads classes not located via Reflection.getCallerClass.
      * @param lastLoader The ClassLoader that loaded the Class that called this Class.
      * @param className The name of the Class.
@@ -473,55 +480,10 @@ public class ThrowableProxy implements S
         return clazz;
     }
 
-    private static void versionCheck() {
-        final Method[] methods = Throwable.class.getMethods();
-        for (final Method method : methods) {
-            if (method.getName().equals("getSuppressed")) {
-                getSuppressed = method;
-            } else if (method.getName().equals("addSuppressed")) {
-                addSuppressed = method;
-            }
-        }
-    }
-
-    /**
-     * Determine if Reflection.getCallerClass is available.
-     */
-    private static void setupCallerCheck() {
-        try {
-            final ClassLoader loader = Loader.getClassLoader();
-            // Use wildcard to avoid compile-time reference.
-            final Class<?> clazz = loader.loadClass("sun.reflect.Reflection");
-            final Method[] methods = clazz.getMethods();
-            for (final Method method : methods) {
-                final int modifier = method.getModifiers();
-                if (method.getName().equals("getCallerClass") && Modifier.isStatic(modifier)
&&
-                    method.getParameterTypes().length == 1) {
-                    getCallerClass = method;
-                    return;
-                }
-            }
-        } catch (final ClassNotFoundException cnfe) {
-            LOGGER.debug("sun.reflect.Reflection is not installed");
-        }
-
-        try {
-            final PrivateSecurityManager mgr = new PrivateSecurityManager();
-            if (mgr.getClasses() != null) {
-                securityManager = mgr;
-            } else {
-                // This shouldn't happen.
-                LOGGER.error("Unable to obtain call stack from security manager");
-            }
-        } catch (final Exception ex) {
-            LOGGER.debug("Unable to install security manager", ex);
-        }
-    }
-
     public ThrowableProxy[] getSuppressed() {
-        if (getSuppressed != null) {
+        if (GET_SUPPRESSED != null) {
             try {
-                return (ThrowableProxy[]) getSuppressed.invoke(throwable);
+                return (ThrowableProxy[]) GET_SUPPRESSED.invoke(throwable);
             } catch (final Exception ignore) {
                 return null;
             }
@@ -530,11 +492,11 @@ public class ThrowableProxy implements S
     }
 
     private void setSuppressed(final Throwable throwable) {
-        if (getSuppressed != null && addSuppressed != null) {
+        if (GET_SUPPRESSED != null && ADD_SUPPRESSED != null) {
             try {
-                final Throwable[] array = (Throwable[]) getSuppressed.invoke(throwable);
+                final Throwable[] array = (Throwable[]) GET_SUPPRESSED.invoke(throwable);
                 for (final Throwable t : array) {
-                    addSuppressed.invoke(this, new ThrowableProxy(t));
+                    ADD_SUPPRESSED.invoke(this, new ThrowableProxy(t));
                 }
             } catch (final Exception ignore) {
                 //

Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java?rev=1519241&r1=1519240&r2=1519241&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
(original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
Sun Sep  1 03:50:20 2013
@@ -17,8 +17,6 @@
 package org.apache.logging.log4j.core.selector;
 
 import java.lang.ref.WeakReference;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -32,6 +30,7 @@ import java.util.concurrent.atomic.Atomi
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.helpers.Loader;
 import org.apache.logging.log4j.core.impl.ContextAnchor;
+import org.apache.logging.log4j.core.impl.ReflectiveCallerClassUtility;
 import org.apache.logging.log4j.status.StatusLogger;
 
 /**
@@ -49,9 +48,7 @@ public class ClassLoaderContextSelector 
 
     private static final AtomicReference<LoggerContext> CONTEXT = new AtomicReference<LoggerContext>();
 
-    private static PrivateSecurityManager securityManager;
-
-    private static Method getCallerClass;
+    private static final PrivateSecurityManager SECURITY_MANAGER;
 
     private static final StatusLogger LOGGER = StatusLogger.getLogger();
 
@@ -59,7 +56,23 @@ public class ClassLoaderContextSelector 
         new ConcurrentHashMap<String, AtomicReference<WeakReference<LoggerContext>>>();
 
     static {
-        setupCallerCheck();
+        if (ReflectiveCallerClassUtility.isSupported()) {
+            SECURITY_MANAGER = null;
+        } else {
+            PrivateSecurityManager securityManager;
+            try {
+                securityManager = new PrivateSecurityManager();
+                if (securityManager.getCaller(ClassLoaderContextSelector.class.getName())
== null) {
+                    // This shouldn't happen.
+                    securityManager = null;
+                    LOGGER.error("Unable to obtain call stack from security manager.");
+                }
+            } catch (final Exception e) {
+                securityManager = null;
+                LOGGER.debug("Unable to install security manager", e);
+            }
+            SECURITY_MANAGER = securityManager;
+        }
     }
 
     @Override
@@ -79,13 +92,12 @@ public class ClassLoaderContextSelector 
         } else if (loader != null) {
             return locateContext(loader, configLocation);
         } else {
-            if (getCallerClass != null) {
+            if (ReflectiveCallerClassUtility.isSupported()) {
                 try {
                     Class<?> clazz = Class.class;
                     boolean next = false;
                     for (int index = 2; clazz != null; ++index) {
-                        final Object[] params = new Object[] {index};
-                        clazz = (Class<?>) getCallerClass.invoke(null, params);
+                        clazz = ReflectiveCallerClassUtility.getCaller(index);
                         if (clazz == null) {
                             break;
                         }
@@ -105,8 +117,8 @@ public class ClassLoaderContextSelector 
                 }
             }
 
-            if (securityManager != null) {
-                final Class<?> clazz = securityManager.getCaller(fqcn);
+            if (SECURITY_MANAGER != null) {
+                final Class<?> clazz = SECURITY_MANAGER.getCaller(fqcn);
                 if (clazz != null) {
                     final ClassLoader ldr = clazz.getClassLoader() != null ? clazz.getClassLoader()
:
                         ClassLoader.getSystemClassLoader();
@@ -130,8 +142,8 @@ public class ClassLoaderContextSelector 
             if (name != null) {
                 try {
                     return locateContext(Loader.loadClass(name).getClassLoader(), configLocation);
-                } catch (final ClassNotFoundException ex) {
-                    //System.out.println("Could not load class " + name);
+                } catch (final ClassNotFoundException ignore) {
+                    //this is ok
                 }
             }
             final LoggerContext lc = ContextAnchor.THREAD_CONTEXT.get();
@@ -219,29 +231,6 @@ public class ClassLoaderContextSelector 
         return ctx;
     }
 
-    private static void setupCallerCheck() {
-        try {
-            final ClassLoader loader = Loader.getClassLoader();
-            final Class<?> clazz = loader.loadClass("sun.reflect.Reflection");
-            final Method[] methods = clazz.getMethods();
-            for (final Method method : methods) {
-                final int modifier = method.getModifiers();
-                if (method.getName().equals("getCallerClass") && Modifier.isStatic(modifier))
{
-                    getCallerClass = method;
-                    break;
-                }
-            }
-        } catch (final ClassNotFoundException cnfe) {
-            LOGGER.debug("sun.reflect.Reflection is not installed");
-        }
-        try {
-            securityManager = new PrivateSecurityManager();
-        } catch (final Exception ex) {
-            ex.printStackTrace();
-            LOGGER.debug("Unable to install security manager", ex);
-        }
-    }
-
     private LoggerContext getDefault() {
         final LoggerContext ctx = CONTEXT.get();
         if (ctx != null) {

Copied: logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ReflectionComparison.java
(from r1519188, logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/ReflectionComparison.java)
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ReflectionComparison.java?p2=logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ReflectionComparison.java&p1=logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/ReflectionComparison.java&r1=1519188&r2=1519241&rev=1519241&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/ReflectionComparison.java
(original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ReflectionComparison.java
Sun Sep  1 03:50:20 2013
@@ -14,70 +14,40 @@
  * See the license for the specific language governing permissions and
  * limitations under the license.
  */
-package org.apache.logging.log4j;
+package org.apache.logging.log4j.core.impl;
 
 import org.apache.logging.log4j.core.Timer;
-import org.apache.logging.log4j.core.helpers.Loader;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.StringFormattedMessage;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 import sun.reflect.Reflection;
 
 import java.lang.reflect.Constructor;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertEquals;
 
 /**
  * Tests the cost of invoking Reflection.getCallerClass via reflection vs calling it directly.
  */
 public class ReflectionComparison {
 
-    private static Method getCallerClass;
-
     private static final int COUNT = 1000000;
 
-    private static Class<?>[] paramTypes = new Class<?>[] {String.class, Object[].class};
-
-    @BeforeClass
-    public static void setupCallerCheck() {
-        try {
-            final ClassLoader loader = Loader.getClassLoader();
-            final Class<?> clazz = loader.loadClass("sun.reflect.Reflection");
-            final Method[] methods = clazz.getMethods();
-            for (final Method method : methods) {
-                final int modifier = method.getModifiers();
-                if (method.getName().equals("getCallerClass") && Modifier.isStatic(modifier))
{
-                    getCallerClass = method;
-                    break;
-                }
-            }
-        } catch (final ClassNotFoundException cnfe) {
-            cnfe.printStackTrace();
-            throw new RuntimeException(cnfe);
-        }
-    }
-
     @Test
-    public void test1() {
+    public void testReflection() {
         final Timer timer = new Timer("Reflection", COUNT);
         timer.start();
-        final Object[] arr = new Object[1];
-        arr[0] = 3;
         for (int i= 0; i < COUNT; ++i) {
-            getCallerClass(arr);
+            ReflectiveCallerClassUtility.getCaller(3);
         }
         timer.stop();
         System.out.println(timer.toString());
     }
 
-
     @Test
-    public void test2() {
-        final Timer timer = new Timer("Reflection", COUNT);
+    public void testDirectly() {
+        final Timer timer = new Timer("Directly", COUNT);
         timer.start();
         for (int i= 0; i < COUNT; ++i) {
 
@@ -87,14 +57,34 @@ public class ReflectionComparison {
         System.out.println(timer.toString());
     }
 
+    @Test
+    public void testBothMethodsReturnTheSame() {
+        assertEquals("1 is not the same.",
+                Reflection.getCallerClass(1 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(1));
+        assertEquals("2 is not the same.",
+                Reflection.getCallerClass(2 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(2));
+        assertEquals("3 is not the same.",
+                Reflection.getCallerClass(3 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(3));
+        assertEquals("4 is not the same.",
+                Reflection.getCallerClass(4 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(4));
+        assertEquals("5 is not the same.",
+                Reflection.getCallerClass(5 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(5));
+        assertEquals("6 is not the same.",
+                Reflection.getCallerClass(6 + ReflectiveCallerClassUtility.JAVA_7U25_COMPENSATION_OFFSET),
+                ReflectiveCallerClassUtility.getCaller(6));
+    }
 
     @Test
-    public void createObjects() throws Exception {
-        Timer timer = new Timer("NewObject", COUNT);
+    public void testCreateObjects() throws Exception {
+        Timer timer = new Timer("CreatObjects", COUNT);
         timer.start();
-        Message msg;
         for (int i = 0; i < COUNT; ++i) {
-            msg = new StringFormattedMessage("Hello %1", i);
+            new StringFormattedMessage("Hello %1", i);
         }
         timer.stop();
         System.out.println(timer.toString());
@@ -110,21 +100,8 @@ public class ReflectionComparison {
         System.out.println(timer.toString());
     }
 
-    private Class<?> getCallerClass(final Object[] array) {
-        if (getCallerClass != null) {
-            try {
-                /*Object[] params = new Object[]{index}; */
-                return (Class<?>) getCallerClass.invoke(null, array);
-            } catch (final Exception ex) {
-                fail(ex.getMessage());
-                // logger.debug("Unable to determine caller class via Sun Reflection", ex);
-            }
-        }
-        return null;
-    }
-
     private Message createMessage(final Class<? extends Message> clazz, final String
msg, final Object... params) throws Exception {
-        final Constructor<? extends Message> constructor = clazz.getConstructor(paramTypes);
+        final Constructor<? extends Message> constructor = clazz.getConstructor(String.class,
Object[].class);
         return constructor.newInstance(msg, params);
     }
 

Modified: logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java?rev=1519241&r1=1519240&r2=1519241&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java
(original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java
Sun Sep  1 03:50:20 2013
@@ -36,6 +36,6 @@ public class ThrowableProxyTest {
         final ThrowableProxy proxy = new ThrowableProxy(throwable);
         final StackTracePackageElement[] callerPackageData = proxy.resolvePackageData(stack,
map, null,
             throwable.getStackTrace());
-        Assert.assertNotNull("No package data returned", callerPackageData);;
+        Assert.assertNotNull("No package data returned", callerPackageData);
     }
 }

Modified: logging/log4j/log4j2/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?rev=1519241&r1=1519240&r2=1519241&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/src/changes/changes.xml (original)
+++ logging/log4j/log4j2/trunk/src/changes/changes.xml Sun Sep  1 03:50:20 2013
@@ -21,6 +21,10 @@
   </properties>
   <body>
     <release version="2.0-beta9" date="soon, very soon" description="Bug fixes and enhancements">
+      <action issue="LOG4J2-322" dev="nickwilliams" type="fix">
+        Centralized reflective use of Reflection#getCallerClass and properly handled its
instability in various versions
+        of Java.
+      </action>
       <action issue="LOG4J2-293" dev="nickwilliams" type="fix" due-to="Abhinav Shah">
         Changed the ConfigurationFactory to recognize and properly use the classpath: URI
scheme in addition to the
         classloader: URI scheme.



Mime
View raw message