bigdataviewer / bigdataviewer-core

ImgLib2-based viewer for registered SPIM stacks and more
BSD 2-Clause "Simplified" License
33 stars 35 forks source link

Multiple thread deadlocks with `addCard` method #143

Closed NicoKiaru closed 4 months ago

NicoKiaru commented 2 years ago

Hi!

I have many thread deadlocks happening with the recent update to pom-scijava 32/33. Bdv is nearly unusable if you want to add extra cards.

Here's a typical trace obtained with visualvm:

Java stack information for the threads listed above:
===================================================
"SciJava-301d8120-Thread-9":
        at java.awt.KeyboardFocusManager.clearMostRecentFocusOwner(KeyboardFocusManager.java:1882)
        - waiting to lock <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at java.awt.Component.setFocusable(Component.java:7144)
        at bdv.ui.CardPanel$Card$HeaderPanel.<init>(CardPanel.java:310)
        at bdv.ui.CardPanel$Card.<init>(CardPanel.java:259)
        at bdv.ui.CardPanel.addCard(CardPanel.java:141)
        - locked <0x0000000776c535d0> (a bdv.ui.CardPanel)
        at bdv.ui.CardPanel.addCard(CardPanel.java:153)
        at ch.epfl.biop.scijava.command.source.register.WarpyEditRegistrationCommand.waitForBigWarp(WarpyEditRegistrationCommand.java:219)
        at ch.epfl.biop.scijava.command.source.register.WarpyEditRegistrationCommand.performBigWarpEdition(WarpyEditRegistrationCommand.java:173)
        at ch.epfl.biop.scijava.command.source.register.WarpyEditRegistrationCommand.run(WarpyEditRegistrationCommand.java:76)
        at org.scijava.command.CommandModule.run(CommandModule.java:196)
        at org.scijava.module.ModuleRunner.run(ModuleRunner.java:163)
        at org.scijava.module.ModuleRunner.call(ModuleRunner.java:124)
        at org.scijava.module.ModuleRunner.call(ModuleRunner.java:63)
        at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:225)
        at org.scijava.thread.DefaultThreadService$$Lambda$196/1791047055.call(Unknown Source)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
"AWT-EventQueue-0":
        at bdv.ui.CardPanel$Card.isCardAboveExpanded(CardPanel.java:466)
        - waiting to lock <0x0000000776c535d0> (a bdv.ui.CardPanel)
        at bdv.ui.CardPanel$Card.access$600(CardPanel.java:234)
        at bdv.ui.CardPanel$Card$HeaderPanel.paintComponent(CardPanel.java:367)
        at javax.swing.JComponent.paint(JComponent.java:1056)
        at bdv.ui.CardPanel$Card$HeaderPanel.paint(CardPanel.java:357)
        at javax.swing.JComponent.paintChildren(JComponent.java:889)
        - locked <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at javax.swing.JComponent.paint(JComponent.java:1065)
        at javax.swing.JComponent.paintChildren(JComponent.java:889)
        - locked <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at javax.swing.JComponent.paint(JComponent.java:1065)
        at javax.swing.JComponent.paintChildren(JComponent.java:889)
        - locked <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at javax.swing.JComponent.paint(JComponent.java:1065)
        at javax.swing.JViewport.paint(JViewport.java:728)
        at javax.swing.JComponent.paintChildren(JComponent.java:889)
        - locked <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at javax.swing.JComponent.paint(JComponent.java:1065)
        at javax.swing.JComponent.paintChildren(JComponent.java:889)
        - locked <0x00000006c38592d0> (a java.awt.Component$AWTTreeLock)
        at javax.swing.JSplitPane.paintChildren(JSplitPane.java:1047)
        at javax.swing.JComponent.paint(JComponent.java:1065)
        at javax.swing.JComponent.paintToOffscreen(JComponent.java:5210)
        at javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1579)
        at javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1502)
        at javax.swing.RepaintManager.paint(RepaintManager.java:1272)
        at javax.swing.JComponent._paintImmediately(JComponent.java:5158)
        at javax.swing.JComponent.paintImmediately(JComponent.java:4969)
        at javax.swing.RepaintManager$4.run(RepaintManager.java:831)
        at javax.swing.RepaintManager$4.run(RepaintManager.java:814)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
        at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:814)
        at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:789)
        at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:738)
        at javax.swing.RepaintManager.access$1200(RepaintManager.java:64)
        at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1732)
        at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
        at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
        at java.awt.EventQueue.access$500(EventQueue.java:97)
        at java.awt.EventQueue$3.run(EventQueue.java:709)
        at java.awt.EventQueue$3.run(EventQueue.java:703)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
        at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

There's another one in the forum post: https://forum.image.sc/t/multiplexed-image-registration-and-combine-using-warpy-and-imagecombiner/71404/27

edit: @tpietzsch, I just saw your message in the forum. Ok, I'll add SwingUtilities invokeLater, but that's a lot of changes... I guess we should add the same comment to splitpanel methods (and bdv close ?). But should it be the same for adding sources ?

NicoKiaru commented 2 years ago

Pfff... There's a lot of issues with FlatLaf, I think some are not bdv related:

Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
    at javax.swing.plaf.basic.BasicScrollBarUI.layoutVScrollbar(BasicScrollBarUI.java:662)
    at javax.swing.plaf.basic.BasicScrollBarUI.layoutContainer(BasicScrollBarUI.java:866)
    at java.awt.Container.layout(Container.java:1513)
    at java.awt.Container.doLayout(Container.java:1502)
    at java.awt.Container.validateTree(Container.java:1698)
    at java.awt.Container.validateTree(Container.java:1707)
    at java.awt.Container.validateTree(Container.java:1707)
    at java.awt.Container.validateTree(Container.java:1707)
    at java.awt.Container.validateTree(Container.java:1707)
    at java.awt.Container.validate(Container.java:1633)
    at javax.swing.RepaintManager$3.run(RepaintManager.java:711)
    at javax.swing.RepaintManager$3.run(RepaintManager.java:709)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at javax.swing.RepaintManager.validateInvalidComponents(RepaintManager.java:708)
    at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1731)
    at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
    at com.formdev.flatlaf.ui.FlatMenuUI.getPreferredMenuItemSize(FlatMenuUI.java:204)
    at javax.swing.plaf.basic.BasicMenuItemUI.getPreferredSize(BasicMenuItemUI.java:376)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1662)
    at com.formdev.flatlaf.ui.FlatMenuUI.getMinimumSize(FlatMenuUI.java:199)
    at javax.swing.JComponent.getMinimumSize(JComponent.java:1744)
    at javax.swing.BoxLayout.checkRequests(BoxLayout.java:483)
    at javax.swing.BoxLayout.preferredLayoutSize(BoxLayout.java:301)
    at javax.swing.plaf.basic.DefaultMenuLayout.preferredLayoutSize(DefaultMenuLayout.java:60)
    at java.awt.Container.preferredSize(Container.java:1799)
    at java.awt.Container.getPreferredSize(Container.java:1783)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
    at com.formdev.flatlaf.ui.FlatTitlePane$2.getPreferredSize(FlatTitlePane.java:187)
    at javax.swing.BoxLayout.checkRequests(BoxLayout.java:484)
    at javax.swing.BoxLayout.preferredLayoutSize(BoxLayout.java:301)
    at java.awt.Container.preferredSize(Container.java:1799)
    at java.awt.Container.getPreferredSize(Container.java:1783)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
    at java.awt.BorderLayout.preferredLayoutSize(BorderLayout.java:714)
    at java.awt.Container.preferredSize(Container.java:1799)
    at java.awt.Container.getPreferredSize(Container.java:1783)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
    at com.formdev.flatlaf.ui.FlatRootPaneUI$FlatRootLayout.layoutContainer(FlatRootPaneUI.java:444)
    at java.awt.Container.layout(Container.java:1513)
    at java.awt.Container.doLayout(Container.java:1502)
    at java.awt.Container.validateTree(Container.java:1698)
    at java.awt.Container.validate(Container.java:1633)
    at javax.swing.RepaintManager$3.run(RepaintManager.java:711)
    at javax.swing.RepaintManager$3.run(RepaintManager.java:709)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at javax.swing.RepaintManager.validateInvalidComponents(RepaintManager.java:708)
    at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1731)
    at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

java.lang.NullPointerException
    at javax.swing.BoxLayout.preferredLayoutSize(BoxLayout.java:302)
    at java.awt.Container.preferredSize(Container.java:1799)
    at java.awt.Container.getPreferredSize(Container.java:1783)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
    at java.awt.BorderLayout.preferredLayoutSize(BorderLayout.java:714)
    at java.awt.Container.preferredSize(Container.java:1799)
    at java.awt.Container.getPreferredSize(Container.java:1783)
    at javax.swing.JComponent.getPreferredSize(JComponent.java:1664)
    at com.formdev.flatlaf.ui.FlatRootPaneUI$FlatRootLayout.layoutContainer(FlatRootPaneUI.java:444)
    at java.awt.Container.layout(Container.java:1513)
    at java.awt.Container.doLayout(Container.java:1502)
    at java.awt.Container.validateTree(Container.java:1698)
    at java.awt.Container.validateTree(Container.java:1707)
    at java.awt.Container.validate(Container.java:1633)
    at javax.swing.SwingUtilities.updateComponentTreeUI(SwingUtilities.java:1231)
    at com.formdev.flatlaf.FlatLaf.updateUI(FlatLaf.java:997)
    at org.scijava.ui.swing.laf.SwingLookAndFeelService.setLookAndFeel(SwingLookAndFeelService.java:130)
    at org.scijava.ui.swing.laf.SwingLookAndFeelService.initLookAndFeel(SwingLookAndFeelService.java:85)
    at org.scijava.ui.swing.AbstractSwingUI.createUI(AbstractSwingUI.java:250)
    at org.scijava.ui.AbstractUserInterface.show(AbstractUserInterface.java:82)
    at org.scijava.ui.DefaultUIService.showUI(DefaultUIService.java:166)
    at org.scijava.ui.DefaultUIService.showUI(DefaultUIService.java:151)
    at sc.fiji.bdvpg.ManualRegistrationDemo.main(ManualRegistrationDemo.java:89)
    at sc.fiji.bdvpg.ManualRegistrationDemo.demoRunOk(ManualRegistrationDemo.java:234)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:27)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
tpietzsch commented 2 years ago

I'm not 100% sure, but I think it is cleaner to use invokeLater outside, not inside the CardPanel. I think here it's pretty clear that this is more or less "pure UI code", on the same level as calling a method in JTree or something.

It's difficult where to draw the line. For example, the BigDataViewer class, I don't see as pure UI, so probably I would put SwingUtilities.invokeLater() inside these methods.

NicoKiaru commented 2 years ago

👍 Ok. I'm going to make a static helper class with addCard and removeCard methods that will call the same method within SwingUtilities.invoke. However, I have plenty of weird NPE on windows with Flatlaf, right when the application starts (at ij.ui().showUI()). I may disable it. It could be windows only.

tpietzsch commented 2 years ago

Pfff... There's a lot of issues with FlatLaf, I think some are not bdv related:

Wow. Yes, I was going to ask which OS this is ...

I was quickly going into the source code for where the exceptions happen and it didn't really make sense to me. For example for the last one

java.lang.NullPointerException
    at javax.swing.BoxLayout.preferredLayoutSize(BoxLayout.java:302)
    ...

This is this line in BoxLayout

            size = new Dimension(xTotal.preferred, yTotal.preferred);

and I don't see how xTotal or yTotal could possibly be null here. Very weird.

NicoKiaru commented 2 years ago

There's a bunch of jdk bugs mentioned with flatlaf (https://bugs.openjdk.org/browse/JDK-8079800). But they are solved normally.

It seems that I can avoid issues right now if I start Fiji with: SwingUtilities.invokeAndWait(() -> ij.ui().showUI()) instead of ij.ui().showUI() directly in the main method. This makes sense somehow, but it wasn't necessary before. (ping @ctrueden)

tpietzsch commented 2 years ago

Aha!!

My personal preference for ij.ui().showUI() would be to put the SwingUtilities.invokeAndWait inside ... but, yeah, personal preference ... 🤷‍♂️

ctrueden commented 2 years ago

I agree that putting AWT manipulation code inside of invokeLater or invokeAndWait (depending on the situation) makes sense. It cannot go directly into the DefaultUIService's showUI() method though, because that is UI agnostic, and cannot depend on AWT/Swing stuff, and some UIs might not even be using the same event dispatch thread style. But in the abstract AWT/Swing UI layers (somewhere in scijava-ui-awt or scijava-ui-swing) would be good to do this.

You do have to be careful with invokeAndWait—it will deadlock if you call it from the EDT, IIRC. That's why we have the queue and invoke methods of ThreadService: to abstract that away. Actually, we could potentially make the UIService's showUI() method use queue or invoke, to maintain good separation of concerns. If anyone has time to explore making such changes, that would be awesome.

NicoKiaru commented 4 months ago

When adding card panels with invokeLater, there's no issue anymore since a while, so I'm going to close this.