Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#3819 gui access outside the EDT during jEdit startup

open
nobody
None
5
2013-08-13
2013-08-13
Eric Le Lay
No

seen via the unit test https://jedit.svn.sourceforge.net/svnroot/jedit/tests/Fest/jedit_tests/test/gui/NoEDTViolationTest.java

Exception in thread "Thread-2"
org.fest.swing.exception.EdtViolationException: EDT violation detected
at java.lang.Thread.getStackTrace(Thread.java:1568)
at org.fest.util.StackTraces.stackTraceInCurrentThread(StackTraces.java:49)
at org.fest.swing.edt.CheckThreadViolationRepaintManager.checkThreadViolations(CheckThreadViolationRepaintManager.java:78)
at org.fest.swing.edt.CheckThreadViolationRepaintManager.addDirtyRegion(CheckThreadViolationRepaintManager.java:69)
at org.fest.swing.edt.FailOnThreadViolationRepaintManager.addDirtyRegion(FailOnThreadViolationRepaintManager.java:31)
at javax.swing.JComponent.repaint(JComponent.java:4784)
at java.awt.Component.repaint(Component.java:3286)
at javax.swing.JComponent.setBackground(JComponent.java:2723)
at org.gjt.sp.jedit.gui.SplashScreen.<init>(SplashScreen.java:39)
at org.gjt.sp.jedit.GUIUtilities.showSplashScreen(GUIUtilities.java:1972)
at org.gjt.sp.jedit.jEdit.main(jEdit.java:394)
at org.gjt.sp.jedit.testframework.TestUtils$1.run(TestUtils.java:137)

Discussion

  • Thomas Meyer
    Thomas Meyer
    2013-08-31

    v1

     
  • Thomas Meyer
    Thomas Meyer
    2013-08-31

    How about attached patch?
    GUI access should only happen in EDT.

     
  • Eric Le Lay
    Eric Le Lay
    2013-08-31

    your patch is better than what I proposed in
    http://sourceforge.net/tracker/?func=detail&atid=100588&aid=3614900&group_id=588
    for the SplashScreen part.

    But there is also a call to propertiesChanged that must be guarded (copied here from my patch).
    --- a/org/gjt/sp/jedit/jEdit.java
    +++ b/org/gjt/sp/jedit/jEdit.java
    @@ -537,7 +537,15 @@ public class jEdit
    // other one-time migration services.
    OneTimeMigrationService.execute();

    - propertiesChanged();
    + try{
    + SwingUtilities.invokeAndWait(new Runnable(){
    + public void run(){
    + propertiesChanged();
    + }
    + });
    + }catch(Exception e){
    + Log.log(Log.ERROR, jEdit.class, "Exception propagating p
    roperties!",e);
    + }

    GUIUtilities.advanceSplashProgress("init modes");

    If you have time, please commit this or I submit a patch based on yours tomorrow...
    @GuardedBy is just for developpers or are there automatic checks from the compiler ?
    Thanks,

     
  • Thomas Meyer
    Thomas Meyer
    2013-09-01

    Hi,

    @GuardedBy is proposed in JSR-305 and is currently just an help for the developer, no checks are done currently.
    The change you propose to call propertiesChanged() in the EDT is problematic as it puts a lot of code on another thread.
    For example, the PropertyManager is accessed from the EDT than. The class PropertyManager is not thread-safe at all!
    Any way updated patch attached.

     
  • Thomas Meyer
    Thomas Meyer
    2013-09-01

    v2

     
  • I think only the call to the Splashscreen should be on EDT, don't you think ?
    Also please follow coding rules, opening braces on a new line :)

     
  • Eric Le Lay
    Eric Le Lay
    2013-09-03

    There are things that are triggered by propertiesChanged() that must be run on the EDT.
    Following stacktrace after applying v1 of the patch.
    Also, following an analysis of call hierarchy in eclipse I was under the impression that propertiesChanged() was always called from the EDT. Thomas, what you say is that other methods of PropertiesManager are called outside the EDT ?

    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: org.fest.swing.exception.EdtViolationException: EDT violation detected
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at java.lang.Thread.getStackTrace(Thread.java:1568)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.fest.util.StackTraces.stackTraceInCurrentThread(StackTraces.java:49)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.fest.swing.edt.CheckThreadViolationRepaintManager.checkThreadViolations(CheckThreadViolationRepaintManager.java:78)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.fest.swing.edt.CheckThreadViolationRepaintManager.addDirtyRegion(CheckThreadViolationRepaintManager.java:69)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.fest.swing.edt.FailOnThreadViolationRepaintManager.addDirtyRegion(FailOnThreadViolationRepaintManager.java:31)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JComponent.repaint(JComponent.java:4784)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at java.awt.Component.repaint(Component.java:3286)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JComponent.setBorder(JComponent.java:1791)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.LookAndFeel.installBorder(LookAndFeel.java:228)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.plaf.basic.BasicPopupMenuUI.installDefaults(BasicPopupMenuUI.java:92)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.plaf.basic.BasicPopupMenuUI.installUI(BasicPopupMenuUI.java:81)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JComponent.setUI(JComponent.java:655)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JPopupMenu.setUI(JPopupMenu.java:212)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JPopupMenu.updateUI(JPopupMenu.java:221)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JPopupMenu.<init>(JPopupMenu.java:186)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at javax.swing.JPopupMenu.<init>(JPopupMenu.java:171)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.gui.tray.JEditSwingTrayIcon.<init>(JEditSwingTrayIcon.java:62)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.Reflect.constructObject(Reflect.java:620)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.BSHAllocationExpression.constructObject(BSHAllocationExpression.java:123)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.BSHAllocationExpression.objectAllocation(BSHAllocationExpression.java:114)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.BSHAllocationExpression.eval(BSHAllocationExpression.java:62)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.BSHPrimaryExpression.eval(BSHPrimaryExpression.java:102)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.BSHPrimaryExpression.eval(BSHPrimaryExpression.java:47)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.Interpreter.eval(Interpreter.java:644)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.Interpreter.eval(Interpreter.java:738)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.bsh.Interpreter.eval(Interpreter.java:727)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.BeanShellFacade._eval(BeanShellFacade.java:148)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.BeanShellFacade.eval(BeanShellFacade.java:113)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.BeanShell.eval(BeanShell.java:377)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.ServiceManager$Descriptor.getInstance(ServiceManager.java:339)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.ServiceManager.getService(ServiceManager.java:265)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.ServiceManager.getService(ServiceManager.java:282)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.gui.tray.JTrayIconManager.addTrayIcon(JTrayIconManager.java:62)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.jEdit.propertiesChanged(jEdit.java:1063)
    [junit] 21:35:17 [Thread-2] [error] BeanShellFacade: at org.gjt.sp.jedit.jEdit.main(jEdit.java:540)