jmeld SwingWorker thread hangs

[WolfgangFahl]

I am not sure about how to handle the EDT/SwingWorker Issues with JMeld. Currently I have a situation where a diff will only show under certain timing conditions. In the real application JMeld hangs on first use. When debugging things work if a few milliseconds time are given but I do not know where yet. So far I found the culprit by pausing the SwingWorker Thread in Eclipse. and the stack shows that line 94 in FileComparison.doInBackground is the culprit.

       if (contentPanel == null) {
                 diffNode.diff();
       }

eventually leads to StatusBar setFont which in line 1893 of Component.java has:

    public void setFont(Font f) {
        Font oldFont, newFont;
        synchronized(getTreeLock()) {

which waits for ever. I would appreciate some usage guidance on how to avoid this.

[albfan]

Any real use case to try to reproduce? I can test on Linux or windows

[WolfgangFahl]

I'll debug and see whether I can get a test case from the results. https://stackoverflow.com/a/1792944/1497139 shows what I am assuming might be the issue.

[WolfgangFahl]

diff --git a/src/main/java/org/jmeld/ui/VersionControlComparison.java b/src/main/java/org/jmeld/ui/VersionControlComparison.java
index d5aa9a9..94446b0 100644
--- a/src/main/java/org/jmeld/ui/VersionControlComparison.java
+++ b/src/main/java/org/jmeld/ui/VersionControlComparison.java
@@ -41,7 +41,11 @@ public class VersionControlComparison extends SwingWorker<String, Object> {
         contentPanel = JMeldPanel.getAlreadyOpen(mainPanel.getTabbedPane(), contentId);
         if (contentPanel == null) {
             diff = new VersionControlDiff(file, DirectoryDiff.Mode.TWO_WAY);
-            diff.diff();
+            SwingUtilities.invokeLater(new Runnable() {
+              public void run() {
+                diff.diff();
+              }
+            });
         }

helps. Doing the same in FileComparison.java is not so easy since ther eis an Exception handler.

[WolfgangFahl]

This also works but the exceptions in the background might not be caught ...

diff --git a/src/main/java/org/jmeld/ui/FileComparison.java b/src/main/java/org/jmeld/ui/FileComparison.java
index fbd6503..34348ab 100644
--- a/src/main/java/org/jmeld/ui/FileComparison.java
+++ b/src/main/java/org/jmeld/ui/FileComparison.java
@@ -64,6 +64,7 @@ public class FileComparison extends SwingWorker<String, Object> {

     @Override
     public String doInBackground() {
+      final Exception[] exs=new Exception[1];
         try {
             if (diffNode == null) {
                 if (StringUtil.isEmpty(leftFile.getName())) {
@@ -91,12 +92,24 @@ public class FileComparison extends SwingWorker<String, Object> {
             contentId = "BufferDiffPanel:" + diffNode.getId();
             contentPanel = JMeldPanel.getAlreadyOpen(mainPanel.getTabbedPane(), contentId);
             if (contentPanel == null) {
-                diffNode.diff();
+              SwingUtilities.invokeLater(new Runnable() {
+                public void run() {
+                  try {
+                    diffNode.diff();
+                  } catch (Exception cex) {
+                    exs[0]=cex;
+                  }
+                }
+              });
             }
-        } catch (Exception ex) {
-            ex.printStackTrace();
-
-            return ex.getMessage();
+        } catch (Exception cex) {
+          exs[0]=cex;
+        }
+        // here we could wait a bit ... e.g. until diffNode is visible
+        if (exs[0]!=null) {
+          Exception ex=exs[0];
+          ex.printStackTrace();
+          return ex.getMessage();
         }

[WolfgangFahl]

I like this better:

diff --git a/src/main/java/org/jmeld/ui/FileComparison.java b/src/main/java/org/jmeld/ui/FileComparison.java
index fbd6503..f96e804 100644
--- a/src/main/java/org/jmeld/ui/FileComparison.java
+++ b/src/main/java/org/jmeld/ui/FileComparison.java
@@ -91,7 +91,11 @@ public class FileComparison extends SwingWorker<String, Object> {
             contentId = "BufferDiffPanel:" + diffNode.getId();
             contentPanel = JMeldPanel.getAlreadyOpen(mainPanel.getTabbedPane(), contentId);
             if (contentPanel == null) {
-                diffNode.diff();
+              SwingUtilities.invokeLater(new Runnable() {
+                public void run() {
+                  diffNode.diff(); 
+                }
+              });
             }
         } catch (Exception ex) {
             ex.printStackTrace();

and in JmDiffNode.diff avoiding to throw an exception with:

 public void diff()
  {
    BufferDocumentIF documentLeft;
    BufferDocumentIF documentRight;
    Object[] left, right;
    StatusBar statusBar = StatusBar.getInstance();

    statusBar.start();
    try {
      documentLeft = null;
      documentRight = null;

      if (nodeLeft != null) {
        documentLeft = nodeLeft.getDocument();
        statusBar.setState("Reading left : %s",
            nodeLeft.getName());
        if (documentLeft != null) {
          documentLeft.read();
        }
      }

      if (nodeRight != null) {
        documentRight = nodeRight.getDocument();
        statusBar.setState("Reading right: %s",
            nodeRight.getName());
        if (documentRight != null) {
          documentRight.read();
        }
      }

      statusBar.setState("Calculating differences");
      diff = new JMDiff();
      left = documentLeft == null ? null : documentLeft.getLines();
      right = documentRight == null ? null : documentRight.getLines();

      revision = diff.diff(left, right, ignore);
      statusBar.setState("Ready calculating differences");
    } catch (Exception ex) {
      statusBar.setState(String.format("Exception: %s -%s"+ex.getClass().getName(),ex.getMessage()));
    }
    StatusBar.getInstance().stop();
  }

[albfan]

As long as it works, seems reasonable to me. I much prefer to discuss code changes on PR (because we can comment there, add new commits), so feel free to open several PR to show different implementations (we can close all them later)

I merged your PR, close this if that fix it.

Capturing the exception from diff, shows we need a log system (info, debug, error, trace) to output traces for this errors (if someone wants to track problems). #58 should fix that

[albfan]

Fixed on e5726a16e684ae76d053eebd2d51330de77268f3