StackOverflow in LogHandler with classloading due to logging in WebAppClassLoader #1563
Comments
oleg-nenashev
changed the title
|
So it will happen if and only if the Debug logging for Jetty is enabled: https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java#L857 It has been added by this commit: afb7505 . |
…LogRecorder#orderedTargets() It is just a hotfix, there may be other LogRecorders affected. Ideally we need a response from Jetty maintainers to jetty/jetty.project#1563. No tests since I see no way to trigger such classloading + no actual need in it.
|
Tough call how to fix. Not sure i like any alternative: leave it to log
implementor to avoid; remove logging from classloader; detect recursion in
classloader; detect recursion in logging.
…On 20 May 2017 12:24, "Oleg Nenashev" ***@***.***> wrote:
Hello,
In Jenkins project we have recently upgraded from Jetty 9.2 to 9.4. In
some conditions we hit StackOverflow in Jetty WebAppClassLoader when Jetty
Debug logging is enabled. Issue - JENKINS-44330
<https://issues.jenkins-ci.org/browse/JENKINS-44330>. It happens, because
one of our LogAppenders does classloading:
at org.eclipse.jetty.util.log.JavaUtilLog.log(JavaUtilLog.java:167)
at org.eclipse.jetty.util.log.JavaUtilLog.debug(JavaUtilLog.java:227)
at org.eclipse.jetty.webapp.WebAppContext.isSystemResource(WebAppContext.java:857)
at org.eclipse.jetty.webapp.WebAppClassLoader.loadClass(WebAppClassLoader.java:548)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at hudson.logging.LogRecorder.orderedTargets(LogRecorder.java:82)
at hudson.logging.LogRecorder$2.publish(LogRecorder.java:108)
at hudson.logging.WeakLogHandler.publish(WeakLogHandler.java:53)
at java.util.logging.Logger.log(Logger.java:738)
at org.eclipse.jetty.util.log.JavaUtilLog.log(JavaUtilLog.java:167)
at org.eclipse.jetty.util.log.JavaUtilLog.debug(JavaUtilLog.java:227)
at org.eclipse.jetty.webapp.WebAppContext.isSystemResource(WebAppContext.java:857)
at org.eclipse.jetty.webapp.WebAppClassLoader.loadClass(WebAppClassLoader.java:548)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at hudson.logging.LogRecorder.orderedTargets(LogRecorder.java:82)
at hudson.logging.LogRecorder$2.publish(LogRecorder.java:108)
at hudson.logging.WeakLogHandler.publish(WeakLogHandler.java:53)
I think we should fix this regression on our side, but I would like to
record this issue here, because it may hit other projects. Jetty project
contributors may want to apply a patch as well.
CC @olamy <https://github.com/olamy> @damphyr <https://github.com/damphyr>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1563>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEUrS7VUj3zQurh4c4cSgiEfvvAowZHks5r7r9LgaJpZM4NhQcl>
.
|
|
We fixed the issue on our side, but recursion prevention would be definitely useful. In the case of Debug logging it should also prevent issues with performance though #1448 won't be addressed then |
|
So if we detect recursion in classloading, then what do we do? Guess it is because of logging and don't log during the load? Seams a bit contrived? It is much harder for us, perhaps impossible to put recursion detection into logging, because we don't implement all the log handlers and it is a big tax to pay on log handlers that can never be recursive. Still not convinced we can do anything reasonable. |
|
Would it be possible to put the recursion check in |
|
Not really:
Then what? return null? throw exception? if so who catches it? How is logging ever instantiated because it can also happen:
Basically logging can't be implemented in terms of anything that actually does logging! Perhaps that means the ClassLoader should not do any logging? |
…LogRecorder#orderedTargets() It is just a hotfix, there may be other LogRecorders affected. Ideally we need a response from Jetty maintainers to jetty/jetty.project#1563. No tests since I see no way to trigger such classloading + no actual need in it.
…LogRecorder#orderedTargets() It is just a hotfix, there may be other LogRecorders affected. Ideally we need a response from Jetty maintainers to jetty/jetty.project#1563. No tests since I see no way to trigger such classloading + no actual need in it.
Hello,
In Jenkins project we have recently upgraded from Jetty 9.2 to 9.4. In some conditions we hit StackOverflow in Jetty WebAppClassLoader when Jetty Debug logging is enabled. Issue - JENKINS-44330. It happens, because one of our LogAppenders does classloading:
I think we should fix this regression on our side. OTOH I would like to record this issue here, because it may hit other projects. Jetty project contributors may want to apply a patch as well.
CC @olamy @damphyr
The text was updated successfully, but these errors were encountered: