I don't understand ClassPackagingDataCalculator :p

Hi Ceki. I was just checking out ClassPackagingDataCalculator because I wanted to see what the warning [WARNING] [,,]\logback-classic\src\main\java\ch\qos\logback\classic\spi\ClassPackagingDataCalculator.java:[17,18] sun.reflect.Reflection is Sun proprietary API and may be removed in a future release was about and I have to admit that I don't get it. What are you doing with the caller class? What would be the problem of just Class.forName for the STE class names? This isn't meant as critique, I would just like to understand what the code does, and why ;) Regards, Joern.

Hi Joern, Only the name of the class is available in StackTraceElement, not the actual class. The Class.forName(String className) method is not guaranteed to get the correct results, especially if the class in question can be found in multiple classloaders. Sun's internal API is used to determine with *certainty* the class which corresponds to the class in the stack frame. Joern Huxhorn wrote:
Hi Ceki.
I was just checking out ClassPackagingDataCalculator because I wanted to see what the warning
[snip]
Regards, Joern.
-- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch

Thorbjørn Ravn Andersen wrote:
Ceki Gulcu skrev:
classloaders. Sun's internal API is used to determine with *certainty* the class which corresponds to the class in the stack frame.
If you use Sun internal API's it most likely does not work on non-Sun JVM's. That might not be what you want to do.
Good point. However, I tested with IBM's JDK and BEA's jrockit. It seems to work fine. -- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch

Ceki Gulcu wrote:
Thorbjørn Ravn Andersen wrote:
Ceki Gulcu skrev:
classloaders. Sun's internal API is used to determine with *certainty* the class which corresponds to the class in the stack frame.
If you use Sun internal API's it most likely does not work on non-Sun JVM's. That might not be what you want to do.
Good point. However, I tested with IBM's JDK and BEA's jrockit. It seems to work fine.
Ah, ok. I think I see... It's because otherwise Class.forName, called from inside the Logger, would use the Loggers ClassLoader to load the class in question. And since the Logger class might be loaded by a container this could result in wrong version information, right? Did this problem actually happen or is this a precaution? Shouldn't e.g. web-applications always execute in an isolated manner? That's my experience, at least, because otherwise a logback configuration in one webapp would overwrite that of another one, wouldn't it? Concerning the use of the internal class, it's really quite unfortunate that functionality like this isn't somehow accessible. Similar functionality is available in private static native Class[] ResourceBundle.getClassContext() so that method, in combination with setAccessible(true), could be used as an alternative to public static native Class Reflection.getCallerClass(int i); but it's just choosing between two evils. The most secure solution would probably be to first try to call sun.reflect.Reflection.getCallerClass using reflection so it's possible to catch Exception if the class or method isn't available (anymore). If that doesn't work you could try, again using reflection, to retrieve the ResourceBundle and make that method accessible. This can fail because of security exception or if the internals of ResourceBundle change. In that case it just isn't possible to resolve really correct ClassLoader. All this reflection would only need to be done once statically in ClassPackagingDataCalculator so the performance impact should be nearly zero. I guess that the method in Reflection is maybe faster since it only returns a single class instead of an array containing all classes in the call stack so I would keep using that as long as it's possible. Regards, Joern.

Comments inline. Joern Huxhorn wrote:
It's because otherwise Class.forName, called from inside the Logger, would use the Loggers ClassLoader to load the class in question. And since the Logger class might be loaded by a container this could result in wrong version information, right?
Yes, that is one scenario. There are other scenarios where multiple class loaders may be in play. If we are going to report information, then we must darn make sure that it is correct. Otherwise, we might be misleading the user. (Wrong information is worse than no information.)
Did this problem actually happen or is this a precaution? Shouldn't e.g. web-applications always execute in an isolated manner? That's my experience, at least, because otherwise a logback configuration in one webapp would overwrite that of another one, wouldn't it?
Oh, it happens quite frequently. There are many different models for the class loader tress in (Java) containers. Moreover, logback and log4j support repository selectors which allow a single instance of logback (or log4j) to offer a distinct logger contexts for each web-application. See the chapter on context selectors in the logback documentation.
Concerning the use of the internal class, it's really quite unfortunate that functionality like this isn't somehow accessible. Similar functionality is available in private static native Class[] ResourceBundle.getClassContext() so that method, in combination with setAccessible(true), could be used as an alternative to public static native Class Reflection.getCallerClass(int i); but it's just choosing between two evils.
I totally agree.
The most secure solution would probably be to first try to call sun.reflect.Reflection.getCallerClass using reflection so it's possible to catch Exception if the class or method isn't available (anymore).
Runtime (but not compile-time) compatibility with JDK lacking Reflection.getCallerClass can be achieved by conditional invocation of getCallerClass(). If it is available, we invoke, otherwise we just skip that call. Compile-time compatibility can be achieved using reflection.
If that doesn't work you could try, again using reflection, to retrieve the ResourceBundle and make that method accessible. This can fail because of security exception or if the internals of ResourceBundle change. In that case it just isn't possible to resolve really correct ClassLoader.
All this reflection would only need to be done once statically in ClassPackagingDataCalculator so the performance impact should be nearly zero.
Good idea.
I guess that the method in Reflection is maybe faster since it only returns a single class instead of an array containing all classes in the call stack so I would keep using that as long as it's possible.
Yah, I am also not too keen on calling a method declared as private via reflection, especially since an equivalent exists. Anyway, before the next release is made we should ensure runtime compatibility with all JDKs, even those lacking the aforementioned method. I'd like to measure the performance impact of invoking getCallerClass through reflection. More interestingly, reflection warps the caller stack so the existing code *might* need to be modified and might become trickier. Cheers, -- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch

Ceki Gulcu skrev:
Thorbjørn Ravn Andersen wrote:
Ceki Gulcu skrev:
classloaders. Sun's internal API is used to determine with *certainty* the class which corresponds to the class in the stack frame.
If you use Sun internal API's it most likely does not work on non-Sun JVM's. That might not be what you want to do.
Good point. However, I tested with IBM's JDK and BEA's jrockit. It seems to work fine.
I believe they both use the Sun Java sources for the runtime (but probably not for the JVM). Have you tried with a JVM that use GNU Classpath? /Thorbjørn
participants (3)
-
Ceki Gulcu
-
Joern Huxhorn
-
Thorbjørn Ravn Andersen