Description

ctrueden

opened on Nov 30, 2020

Issue body actions

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

Likely a bug in the SciJava logging framework.

CC @kephale @skalarproduktraum @maarzt

Activity

1. imagejan commented on Nov 30, 2020

   

   imagejan

   on Nov 30, 2020

   Member

   More actions

   What was the script run from the TextEditor to trigger this issue?
2. ctrueden commented on Nov 30, 2020

   

   ctrueden

   on Nov 30, 2020

   MemberAuthor

   More actions

   # @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.
3. ctrueden commented on Nov 30, 2020

   

   ctrueden

   on Nov 30, 2020

   MemberAuthor

   More actions

   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.
4. imagejan commented on Nov 30, 2020

   

   imagejan

   on Nov 30, 2020

   Member

   More actions

   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
   
5. ctrueden commented on Nov 30, 2020

   

   ctrueden

   on Nov 30, 2020

   MemberAuthor

   More actions

   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).
6. maarzt commented on Dec 3, 2020

   

   maarzt

   on Dec 3, 2020

   Contributor

   More actions

   I would suggest this fix #411

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

   Reacted by Jan Eglinger
7. skalarproduktraum commented on Dec 3, 2020

   

   skalarproduktraum

   on Dec 3, 2020

   Member

   More actions

   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 😏
8. maarzt commented on Dec 4, 2020

   

   maarzt

   on Dec 4, 2020

   Contributor

   More actions

   mean-spirited entity

   Sounds like a conspiracy...

   I could confirm that #411 fixes the bug. The exception can be triggered by the following script:

   @LogService log
   log.setLevel("net.imagej", 2)
   log.warn("Hello World!")
9. maarzt commented on Dec 4, 2020

   

   maarzt

   on Dec 4, 2020

   Contributor

   More actions

   @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?
10. imagejan commented on Dec 4, 2020

    

    imagejan

    on Dec 4, 2020

    Member

    More actions

    @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 #403, @LogService without a comment would actually work, but this option was never documented neither intended.

Issue actions