Threads are EVIL
Jesse Eichar
I'm debugging an application with many threads running every which
way. I wondering if anyone has any advice on how to get it under
control and how to determine what classes need to be thread safe. If
it makes a difference I am tied to the Eclipse platform for this.
A couple "evil things" that I have noticed are:
1. The display thread seems to encounter synchronization blocks often.
2. Many threads attempt to interact synchronously with the display
thread since they need to obtain information from widgets or show
progress
3. Listeners are sometimes notified within synchronized blocks
Thanks in advance,
Jesse
Werner Schuster (murphee)
> I'm debugging an application with many threads running every which
> way. I wondering if anyone has any advice on how to get it under
> control
Well... not having them "run any which way" would be a start...
> it makes a difference I am tied to the Eclipse platform for this.
>
What do you mean? Is it an Eclipse plugin (RCP or generally IDE)?
> A couple "evil things" that I have noticed are:
> 1. The display thread seems to encounter synchronization blocks often.
> 2. Many threads attempt to interact synchronously with the display
> thread since they need to obtain information from widgets or show
> progress
>
If you're using SWT syncExec, try to do as little as possible on the
EDT, ie. just go in, do what you have to do and do the rest outside of
the EDT.
If you're just looking for threading tips, I'd use the recently
published Brian Goetz book.
murphee
--
Blog @ http://jroller.com/page/murphee
Maintainer of EclipseShell @ http://eclipse-shell.sourceforge.net/
Jesse Eichar
On 22-Sep-06, at 11:32 AM, Werner Schuster (murphee) wrote:
>
> Jesse Eichar wrote:
>> I'm debugging an application with many threads running every which
>> way. I wondering if anyone has any advice on how to get it under
>> control
>
> Well... not having them "run any which way" would be a start...
Agreed any ideas on how to analyse a program for what threads are
necessary? Or how to trace what threads are running where.
>> it makes a difference I am tied to the Eclipse platform for this.
>>
> What do you mean? Is it an Eclipse plugin (RCP or generally IDE)?
The application is an eclipse RCP application so eclipse is the tool
to use for development.
>
>
>> A couple "evil things" that I have noticed are:
>> 1. The display thread seems to encounter synchronization blocks
>> often.
>> 2. Many threads attempt to interact synchronously with the display
>> thread since they need to obtain information from widgets or show
>> progress
>>
> If you're using SWT syncExec, try to do as little as possible on the
> EDT, ie. just go in, do what you have to do and do the rest outside of
> the EDT.
>
> If you're just looking for threading tips, I'd use the recently
> published Brian Goetz book.
I'll take a look at the book. Threads can be a real head ache.
Something that would be really useful would be a way to "map" the
active threads, maybe a profiler can help with this.
Thanks,
Jesse
Werner Schuster (murphee)
> On 22-Sep-06, at 11:32 AM, Werner Schuster (murphee) wrote:
>
>> Well... not having them "run any which way" would be a start...
>>
> Agreed any ideas on how to analyse a program for what threads are
> necessary? Or how to trace what threads are running where.
>
> Something that would be really useful would be a way to "map" the
> active threads, maybe a profiler can help with this.
>
Depends what you mean by "map";
You do see the threads in the Eclipse debugger view, and you can just
hit suspend at any time to stop them (if you do, all threads will show
their stack frames - you might have to un-collapse them). Is that what
you want to see?
There's a profiler for Eclipse http://www.eclipse.org/tptp/ (or if you
have the proper Update Sites or the Callisto release, just look into the
update manager and look for profiling or testing features) - although
I'm not sure how that's going to help you with threading problems. (They
have something called Dynamic Probes, which allows you instrument your
code easily - if that's something you want).
Rob Wilson
> way. I wondering if anyone has any advice on how to get it under
> control and how to determine what classes need to be thread safe. If
> it makes a difference I am tied to the Eclipse platform for this.
I'm assuming that your using Eclipse simply as an IDE and that you're
not using the Eclipse RCP platform stuff? It's not entirely clear for
example if you're really developing Swing GUI stuff within Eclipse, or
using SWT for Eclipse RCP plugins perhaps?
My general tip is to read the O'reilly book on threads, it covers the
newer Java 5 threading support and does cover some Swing threading
issues.
I would say that if you are manipulating GUI components before they are
realised (displayed), then you are ok to set / read values from them
without needing to consider threading issues. Likewise, if you are
handling an ActionPerformed event, you don't need to do anything
special, because it's already running in the Event Dispatch Thread
(EDT).
The time when it's a problem, is when your not already in the EDT - and
for newbies that's unlikely anyway.
However, as you gain more experience, you will start writing your own
threads that will update the GUI asynchronously. It's times like this
that Swing can bite you hard, but a few general rules should help you.
If you plan to run a long-running thread, it's obviously normal to
create a new thread - and highly recommended to show a perceived speed
improvement and responsiveness in your GUI. Whenever you READ a GUI
component you should read the attributes from the EDT, and this can be
performed in by using SwingUtilities.invokeLater, or invokeAndWait.
If you want to read the value of a swing GUI component and deal with
the returned value outside of the EDT, then I believe you should use
invokeAndWait - but I understand you may want to keep this to a minimum
(I actually don't use this).
If you want to MODIFY the swing component, then you MUST always ensure
you're in the EDT - by using an invokeLater or invokeAndWait.
> 1. The display thread seems to encounter synchronization blocks often.
I assume this is your code with the synchronous blocks - since Swing is
not thread-safe? - I would refrain from using synchronised methods
whenever possible, using alternatives like synchronised blocks based on
an object, or using some of the atomic classes in Java 5.
One tip here, is that if your using primatives that are represented in
16bits or less, then they do not need synchronising.
> 2. Many threads attempt to interact synchronously with the display
> thread since they need to obtain information from widgets or show
> progress
Yep - I've learnt recently that I took things for granted. My recent
trap was assuming that the JTable abstract table models fired events in
the EDT, they DONT! So you may want to ensure that your 'set' methods
in your model do the 'fireXYZ' inside an invokeLater, or override each
fireXYZ to check what thread your in first - and take the appropriate
action.
> 3. Listeners are sometimes notified within synchronized blocks
Not sure whether this is correct. I have used Vectors in the past to
ensure that the listener list can be iterated through without getting
concurrent modification exceptions - other times I've made my
getListeners() method return a new collection - so there is no need to
synchronise anything other than perhaps. Oh, and I believe Java5 has
some nice new collection implementation for thread safety that
definately help with scenarios like listener lists (where you mostly
read from the list rather than update them)... Again the O'reilly book
will help here.
> Thanks in advance,
>
> Jesse
No problems, but this comes with a disclaimer because I still cock it
up ;-) In fact, right now I have a similar issue and it's a real pain
to debug... I'd like to know what Romain Guy would recommend when it
comes to updating Swing components via the Models, should the call to
the model be responsible for putting the calls in the EDT, or should
the model?
I vote that the model should be responsible, and then it could do some
neat stuff in one place. My example would be the fireTableChanged
method could be overriden to see whether you're already in the EDT
thread, and if you're not, put it in the EDT but ONLY if the listener
is a particular type of class, i.e. an instanceof java.awt.Component or
javax.swing.TableModel.... then any of your listeners that react to the
model changing would not be added to the EDT... Romain - I'd love to
hear your thoughts ;-)
Rob.
Werner Schuster (murphee)
> Not sure whether this is correct. I have used Vectors in the past to
> ensure that the listener list can be iterated through without getting
> concurrent modification exceptions - other times I've made my
>
after an iterator has been requested will cause it.
Try:
foo = getListWith42Items();
Iterator it = foo.iterator();
it.next();
foo.remove(0);
it.next(); // CME !
The fact that Vector methods are synchronized doesn't help here. There's
an entry on this in the Swing Hacks book on how this can cause problems
with event dispatching code that uses Iterators to iterate over Listener
lists.
murphee
Rob Wilson
> after an iterator has been requested will cause it.
I use a ListIterator for that.
> The fact that Vector methods are synchronized doesn't help here. There's
> an entry on this in the Swing Hacks book on how this can cause problems
> with event dispatching code that uses Iterators to iterate over Listener
> lists.
The example below shows that I can add items to the vector without
causing a CME
import java.util.ListIterator;
import java.util.Vector;
public class VectorExample {
public static void main(String[] args) {
Vector<String> v = new Vector<String>();
v.add("Apple");
v.add("Orange");
v.add("Pears");
ListIterator<String> i = v.listIterator();
while (i.hasNext()){
String s = i.next();
i.add("CME Free Zone");
System.out.println(s+" size of v:"+v.size());
}
}
}
The output will then display
Apple size of v:4
Orange size of v:5
Pears size of v:6
But this is only useful if you are iterating through a vector and want
to add or remove elements from the list at the same time (and you know
that no other threads could be updating it).
But here is another example that uses list iterators combined with the
synchronised keyword on the Vector itself to have 100 threads updating
and reading the vector at the same time. It would be important to
place the same synchronised block around any other accesses on the
vector too...
import java.util.ListIterator;
import java.util.Vector;
public class VectorExample {
Vector<String> v = new Vector<String>();
public static void main(String[] args) {
VectorExample ve = new VectorExample();
}
VectorExample() {
// Start 100 threads that all try to update the vector
for (int threadIndex=0; threadIndex<100; threadIndex++){
Thread t = new DummyThread(v, this, threadIndex);
t.start();
}
}
}
/**
* Simply runs 100 iterations of adding entries
* into the Vector, with the name of the thread
* being inserted. You can then see that 100
* different threads are updating ok.
*/
class DummyThread extends Thread{
Vector<String> v;
VectorExample ve;
int threadId;
DummyThread(Vector<String> v, VectorExample ve, int threadID){
this.v = v;
this.ve = ve;
this.threadId = threadID;
}
public void run() {
for (int i = 0; i < 100; i++) {
synchronized (v) {
v.add("In thread " + this.getName() + " adding i:" +
i);
// Pretend to display the contents, this will read the
// vector whilst other threads are writing / reading
too.
if (i == 49 || i == 99) {
System.out
.println("Pretending to output the
contents, size = "
+ v.size());
ListIterator<String> it = v.listIterator();
while (it.hasNext()) {
String s = it.next();
// System.out.println(s);
}
}
}
}
}
}
Will
Sounds like you're new to threads in Java. Don't worry - there's not
that much to get your head around and once you do, they are really
useful!
If I'm right and you're just starting out with threads, I recommend you
FIRST read "Java Thread Programming" by Paul Hyde; and SECOND read
"Java Concurrency in Practice" by Brian Goetz et al.
The first is aimed at Java 1.4, but gives a really simple, practical
introduction to the concept of multithreaded programming and how to get
the job done in Java. From what you descibe, it sounds like this is the
level of knowledge you need to figure out what your application is
doing.
Once you understand what the threads in your application are doing, the
second book will take you to Java 5 concurrency, where the new
java.util.concurrent classes almost completely replace the need to work
with low-level concurrency controls (namely volatile and synchronized).
And since you're probably in a hurry and don't have time to read either
book, here are my 6 simple tips:
#1 - Know what each of your threads does, and which threads can be
inside which methods
...You can use asserts to check the names of executing threads in
sensitive methods.
#2 - Know which variables are accessed by multiple threads, and which
lock guards them
...Brian Goetz recommends annotating/commenting your variables to
describe how they are guarded. But the best way to simplify this job is
to avoid variables - use immutable classes wherever you can. (They're
automatically threadsafe, very fast, and easy to code.)
#3 - Remember it's not enough to synchronize the writes to a shared
variable
...You also have to synchronize the reads with the same lock.
#4 - Don't be afraid of the synchronized keyword
...It is NOT evil. Do be afraid however of large stacks of code inside
a synchronized block/method. If you find one, take out the bits that
don't need to be synchronized and/or split it up into smaller
synchronized blocks.
#5 - Avoid nesting locks (ie. synchronized within synchronized)
...Try to think of another way to do it. (There are ways to get away
with it, but you're asking for trouble later down the road.)
#6 - Keep it SIMPLE
...Don't do with 2 threads what you can do with 1. (Unless you get a
measurable performance improvement by parallel processing CPU-intensive
or IO-blocking code.)
As for figuring out what those threads are doing (tip #1), try putting
a breakpoint somewhere you know one of the threads goes, then as you
step through the loop, take note of all the places it goes, and the
locks it acquires (tip #2). Then repeat this with the next thread, etc.
Best of luck and I hope you learn to love threads as much as they love
you!
Cheers,
Will.
Raymond
I've currently working on a highly multi-threaded swing client.
>From what I've found there are two general areas where synchronization
is imporant. The first is when updating the UI; the user needs to
see things in the sequence they happen; and the second is when
reading\writing the underlying data model (if one exists).
My strategy thus-far has been to implement a thread-safe api for
the data model (every method call into the api is thread-safe save
reentrant ones) using the proxy pattern. As well for the UI I use
simple command objects. Let's take a look at a quick UI example.
Let's say you have a status bar element you want to update.
public class StatusBar extends UIContainerWidget {
....
private final Object updatorLock = new Object();
public StatusBarUpdator createUpdator() {
return new StatusBarUpdator(updatorLock, statusLabel);
}
}
public class StatusBarUpdator {
...
StatusBarUpdator(final Object lock, final UILabelWidget
statusLabel) {
this.lock = lock;
this.statusLabel = statusLabel;
}
....
public void writeStatus(final String statusText) {
synchronized (lock) {
statusLabel.setText(statusText);
}
}
....
}
Now from within any of your threads you simply need to grab an
updator from your status panel and write to it. The same pattern can
be applied if you need to read from the UI. In swing specifically
there is a utility class that you can use instead of synchronizing
yourself; however I don't recall if SWT has the same.
With respect to the listeners being notified within synchronized
blocks; I'd be interested in the reason for the synchronization here.
If it's to ensure integrity of the listeners themselves that's fine but
if it's to synchronized the event generation itself; I'd suggest moving
the synchronization to the actual handling of the event instead.
I hope this helps. Shoot me an e-mail if you'd like more details.
Raymond
Steven Bell
Curt Cox
should watch this presentation to learn more about the application
architecture thoughts of the Swing team.
"A Simple Framework for Desktop Applications : TS-3399, 2006"
http://developers.sun.com/learning/javaoneonline/2006/desktop/TS-3399.html
Scott Violet has 3 part series of blog entries about Swing application
architecture.
http://weblogs.java.net/blog/zixle/archive/2006/03/index.html
If you're really daring, you could try out Superficial.
"Superficial is a fresh approach to designing and coding GUI
applications that bridges the gap between high-level architectural
concepts such as Model-View-Controller, and low-level widget toolkits
such as Java Swing."
http://superficial.sourceforge.net/
To me Swing exemplifies Java APIs:
Me: How do I use this exactly?
Java: Any way you want -- it's up to you.
Me: This doesn't seem to be working out too well.
Java: Of course not, it was never intended to be used that way.
Jesse Eichar
[snip]
Now from within any of your threads you simply need to grab an
updator from your status panel and write to it. The same pattern can
be applied if you need to read from the UI. In swing specifically
there is a utility class that you can use instead of synchronizing
yourself; however I don't recall if SWT has the same.
[snip]
Will
> Let's take a look at a quick UI example.
> Let's say you have a status bar element you want to update.
It's an interesting idea, but there are a couple of problems with this
approach.
Locking on updatorLock doesn't achieve anything unless you lock on it
for all READS as well as writes (see my tip #3 earlier in this thread).
That means StatusBar must lock on updatorLock even internally whenever
it accesses its own text property. This is probably the most often
misunderstood rule of Java thread programming. Let me say it again: if
multiple threads work with a variable, you need a lock not just to
TOUCH it, but also to LOOK at it.
So the existence of the StatusBarUpdator class doesn't really add any
value, since the locking in the writeStatus() method could be done
inside StatusBar.setText(). If the intention is to give non-EDT threads
a way to update the widget while still allowing the EDT a non-locking
way to do it, it won't be threadsafe. If other threads are
touching/looking at the text property, unfortunately for the EDT, it
will need to synchronize on the same lock as them.
As Rob and Steven point out, the simplest and most correct way to
update widgets from non-EDT threads is to use invokeLater() or
invokeAndWait(). I'm not sure what the SWT equivalents are but they
must exist. These are a thread-safe way for Thread A to delegate the
update to the EDT, so there need be no locks in the widget code at all.
> With respect to the listeners being notified within synchronized
> blocks; I'd be interested in the reason for the synchronization here.
> If it's to ensure integrity of the listeners themselves that's fine but
> if it's to synchronized the event generation itself; I'd suggest moving
> the synchronization to the actual handling of the event instead.
Agree with you here. But it's important for anyone (else) reading this
to understand that the EDT dispatches the events (hence the name). The
listeners' callback methods will run on EDT horsepower. So the
synchronization you do inside them guards NOT the properties of UI
widgets, but other variables that your non-EDT threads are interested
in.
Will.
Will
> Now
> what I'd like to do is "map" the threads going through certain
> methods that I've marked as "potentially blocking" . My current idea
> has been to add a check at the start of the method to see if it is a
> "Display/UI" thread and at least print the stack trace to see if I
> can determine what code is responsible for entering the method.
I recommend you put asserts at the beginning of "potentially blocking"
methods, with the condition that the current thread is not the EDT, so
you get stack traces for every violation. Then you just go through one
by one and convert each to a pattern of either spawning a new thread,
or submitting a task to an execution service (read Goetz). The method
code should then update the UI with results at the end using the
asyncExec() / syncExec() methods.
> My worry is there may be places where such methods have to be called
> in the display thread... But I'll climb that mountain when I get to it.
Why would you ever need to do that? If it's to prevent further input
while waiting for the "potentially blocking" operation to complete,
it's much nicer for the user if you just disable the inputs you don't
want, but still hand off the execution to a non-EDT thread, then
re-enable inputs when finished. It's even nicer if you leave a "Cancel"
button enabled in case they get sick of waiting. Execution services in
Java 5 make this much easier than it sounds.
Best of luck! Let us know how it goes
Will.