scijava-common icon indicating copy to clipboard operation
scijava-common copied to clipboard

IllegalStateException in logging framework when executing a script

Open ctrueden opened this issue 4 years ago • 10 comments

During the first session of the sciview tutorial at I2K 2020:

sciview-IllegalStateException-I2K-2020

Likely a bug in the SciJava logging framework.

CC @kephale @skalarproduktraum @maarzt

ctrueden avatar Nov 30 '20 16:11 ctrueden

What was the script run from the TextEditor to trigger this issue?

imagejan avatar Nov 30 '20 16:11 imagejan

# @SciView sv

sv.addSphere()

print('My first script!')

On @kephale's Linux box.

Probably not too enlightening though... looks more like a race condition to me. Here's the relevant line:

https://github.com/scijava/script-editor/blob/script-editor-0.5.6/src/main/java/org/scijava/ui/swing/script/TextEditor.java#L2042

It's just a debug log statement during cleanup phase.

ctrueden avatar Nov 30 '20 20:11 ctrueden

Huh.

https://github.com/scijava/scijava-common/blob/scijava-common-2.85.0/src/main/java/org/scijava/log/CallingClassUtils.java#L52-L62

Maybe the Thread.currentThread().getStackTrace() somehow fails, due to the multithreaded stuff going on? Seems like we should fail more gracefully here rather than throwing IllegalStateException.

ctrueden avatar Nov 30 '20 20:11 ctrueden

FWIW, I can reproduce the ClassNotFoundException (that presumably leads to this issue here) using the following Groovy script:

stackTrace = Thread.currentThread().getStackTrace()

className = stackTrace[1].getClassName()
println className

clazz = Class.forName(className)
println clazz

which throws:

java.lang.ClassNotFoundException: java_lang_Thread$getStackTrace$0

imagejan avatar Nov 30 '20 21:11 imagejan

Ahh! I missed that Class.forName was being called. Yeah, that's not good. Thanks for catching that, @imagejan.

If we really need to do dynamic class lookup there, we should change it to Context.getClassLoader().findClass(className).

ctrueden avatar Nov 30 '20 21:11 ctrueden

I would suggest this fix https://github.com/scijava/scijava-common/pull/411

I failed to reproduce the problem. I therefor couldn't make sure that the fix really works.

maarzt avatar Dec 03 '20 15:12 maarzt

Thanks for the fix @maarzt! @kephale's Linux box is possessed by a mean-spirited entity that causes random errors, I suggested to him repeatedly to start fresh with it, therefore I am not totally surprised this is not reproducible 😏

skalarproduktraum avatar Dec 03 '20 15:12 skalarproduktraum

mean-spirited entity

Sounds like a conspiracy...

I could confirm that https://github.com/scijava/scijava-common/pull/411 fixes the bug. The exception can be triggered by the following script:

@LogService log
log.setLevel("net.imagej", 2)
log.warn("Hello World!")

maarzt avatar Dec 04 '20 15:12 maarzt

@ctrueden I needed to change the parameter annotation in the script from @LogService log to #@LogService log when I used the latest version of scijava-common. Is this intentional?

maarzt avatar Dec 04 '20 15:12 maarzt

@maarzt the syntax is either @LogService in a comment (i.e. # or // depending on the script language; that's the "old" syntax), or language-independently #@ LogService (without space between # and @; that's the recommended syntax since 3 years). The latter has the advantage that it can be used anywhere in the script, not only in the script header (so e.g. after a license header etc.).

It's possible that before https://github.com/scijava/scijava-common/pull/403, @LogService without a comment would actually work, but this option was never documented neither intended.

imagejan avatar Dec 04 '20 18:12 imagejan