A bunch of our "legacy" (i.e., old and busted) code is broken in that it accesses Swing GUIs, manipulating them and their data, not on the event dispatch thread. For the most part, things are okay. But there are a number of parts where things just act flakey.

In order to avert this, I put the following bit together -- as far as I can tell, it isn't actually broken (at least in the guise of not working right) but it seems kinda ... wtf-y:

@Override
public void setText(final String msg) {
  if (SwingUtilities.isEventDispatchThread()) {
    super.setText(msg);
  } else {
    SwingUtilities.invokeLater(new Runnable() {
      @Override
      public void run() {
        setText(msg);
      }
  });
}

What do you think?

This is how the Windows Software Compatibility team deals with problems: "Okay, so this program is just using a C cast rather than the appropriate conversion method? Fine then, we'll add an ASM hack that dynamically detects if it looks like the wrong type of object and converts it to the right one."

That's kind of how you're SUPPOSED to do it... in C# the idiom is:

if(GuiObject.InvokeRequired)

   GuiObject.Invoke(...)

else

   GuiObject.SetProperty

This is fine. What's wrong with it?

AngelSL:

This is fine. What's wrong with it?

zelmak:

@Override
public void setText(final String msg) {
  ...
    super.setText(msg);
  ...
}

If XYZ process wants to access a widget, the non-WTF way is for XYZ process to make sure it's in the correct thread and then access the widget.

Zelmak's approach is to replace all widgets with custom widgets that override all the methods, and if you call a method in the wrong thread it'll fix that for you. That's a WTF. It lets you do this without modifying the caller. It might sort-of work. But there will be bugs - e.g. after calling widget.setText("foo") the result of widget.getText() may or may not be "foo", depending on what thread you were in and whether the event thread is busy doing other stuff. If you try to "fix" getters in a similar fashion, then the code may sometimes deadlock (since your thread would have to block and wait for the UI thread). Generally this is a bad idea. A better idea would be:

@Override
public void setText(final String msg) {
    assert(SwingUtilities.isEventDispatchThread());
    super.setText(msg);
}

I think its an ugly hack which may be needed depending on the rest af the code base. But I think you should use invokeAndWait, instead of invokeLater in order to prevent surprises.

With invokeLater calling getText() after calling setText() may return either the old or the new value depending on timing, which may cause all kinds of trouble later. (Just imagine having to make a unit test for these widgets). 

 Well thinking about it: If you also wrap getText in an invokeLater call, you would always get the new value, but I still think you should use invokeAndWait just to prevent timing trouble.

mt@ilovefactory.com:

But I think you should use invokeAndWait, instead of invokeLater in order to prevent surprises

invokeAndWait can cause serious slow-down as well as deadlocks. invokeLater is safer in that regard. Basically, if you need the values to be set now, e.g. if your read them later, use invokeAndWait. Otherwise, invokeLater.

In C# I once had a method that looked like this:

public void InvokeIfRequired(Action action) {
  if (InvokeIsRequired) {
    Invoke(action);
  } else {
    action();
  }
}

You could possibly make something similar so you wouldn't have to copy paste this pattern, though I'm not sure how much that would help in Java.

zelmak:

@Override
public void setText(final String msg) {
  if (SwingUtilities.isEventDispatchThread()) {
    super.setText(msg);
  } else {
    SwingUtilities.invokeLater(new Runnable() {
      @Override
      public void run() {
        setText(msg);
      }
  });
}

What do you think?

i don't know much of Java, and I still am not familiar enough with multithreading patterns in general, but this looks very much like a standard pattern for safely handling cross-thread calls in .net - you check if the function was called from the same thread, if yes, you just do your stuff, and if not, you invoke it. so if i understand correctly, not wtfy at all imo.

 

curtmack:

This is how the Windows Software Compatibility team deals with problems: "Okay, so this program is just using a C cast rather than the appropriate conversion method? Fine then, we'll add an ASM hack that dynamically detects if it looks like the wrong type of object and converts it to the right one."

That's one of the key reasons why Windows is enjoying it's market share, right? Because it's (compentent) developers are covering for bugs that 3rd party program's are bringing in?

Like this?

Nothing wrong with it. Two wrongs make a right.