Thread doesn't interrupt!? HELP please

[contrex]

My program let the user choose a directory.
My program gets all image files of this directory and stores its
references in an array called "images".
Then I start a thread (the one and only) which iterates through the
array in order to create a thumnail for each image. This can take a
while.

When the user selects another directory I interrupt the thread (if
still running) and set it to null BEFORE fetching the new file
references in the array. See method "getImages" below please.

The problem is that the thread hadn't stopped yet! This causes
problems because the thread is still in the loop going from 0 until
images.length and tries to access the array at a position which is no
more valid (assuming the user selected a directory having fewer files
as the previously selected directory and the thread is working on one
of the last images).

The really strange thing is that I see in my log that the thread is
interupted (and even set to null), the thread is not restartet (seeing
in my logs too) and nevertheless the thread accessed the array! How it
is possible??

I even added a parameter to the constructor of "MyThumb" in order to
identify the caller. Each caller has to identify itself. So I see who
is calling the method. And I see it is the thread who calls the method
even if it was setted to null according to my logs and was not
restarted.

Can you explain me this please?
How to resolve this?

Thank for a hint!




Here my simplified code :



public class MyClass extends JFrame{

File []images=null;
Thread updateThread=null;
MyThumb []allThumbs=null;

//method for creating thumbs by a thread
public void updateThumbs() {

if(updateThread == null){
updateThread = new Thread() {
public void run(){
try{
if(images != null){
allThumbs=new MyThumb[images.length];
for(int x=0; x<allThumbs.length; x++){
allThumbs[x]=new MyThumb(images[index]);//CRASH
}
}
}
catch(Exception ex){
ex.printStackTrace(System.out);
}
finally{
updateThread = null;
}
}
};
updateThread.start();
}
}


//get files of an directory
public void getImages(String folder) {

if(updateThread!=null){
updateThread.interrupt();
updateThread = null;
}

File f = new File(folder);
images = f.listFiles(new FileFilter() {...}//get image files
}
}

 

[Chris Smith]

My program let the user choose a directory.
My program gets all image files of this directory and stores its
references in an array called "images".
Then I start a thread (the one and only) which iterates through the
array in order to create a thumnail for each image. This can take a
while.

When the user selects another directory I interrupt the thread (if
still running) and set it to null BEFORE fetching the new file
references in the array. See method "getImages" below please.

The problem is that the thread hadn't stopped yet!

Right. The interrupt() method of Thread will only interrupt certain
interruptible operations. Loosely speaking, interruptible operations
are those that declare InterruptedException in their exception clause.
They include sleeping and waiting on a monitor notification. When you
interrupt a thread that is not performing an interruptible operation,
nothing happens until the next interruptible operation, which will then
be immediately interrupted.

Your problem is that your thread doesn't perform any interruptible
operations, and therefore doesn't ever respond to the interruption at
all. This is something that needs to be dealt with when interrupting
CPU-bound or I/O-bound threads. For a CPU-bound thread, it's a matter
of inserting checks for interruption at good stopping points in the
work. Such a check might look like this:

if (Thread.interrupted()) throw new InterruptedException();

This causes
problems because the thread is still in the loop going from 0 until
images.length and tries to access the array at a position which is no
more valid (assuming the user selected a directory having fewer files
as the previously selected directory and the thread is working on one
of the last images).

This is a more serious problem, though. Even if interruption were made
to work through polling, you still need to make sure that the old thread
really has finished prior to changing the data that it's using. This
would involve using monitor notification in some way, so that you
interrupt the thread and *then* wait for it to tell you that it's done.
For example:

updateThread.interrupt();
while (!updateTask.isFinished()) wait();

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[David Zimmerman]

You're run method never checks to see if the thread is interrupted
(Thread.isInterrupted()) and you never call any interruptable methods
(EG wait, sleep). You need to watch that flag in your inner most loop

 

[Chris Smith]

updateThread.interrupt();
while (!updateTask.isFinished()) wait();

One further comment. You could also use Thread.join here to wait for
the thread to complete, instead of wait/notify and an isFinished method.
That would involve less changes to existing code than my suggestion. I
use the wait/notify method because I tend to avoid dependencies on the
threading model in external code, and I see it as possible in the future
that the task for which you spawn a thread here might get moved to a
thread pool, where an interrupted thread would never really end.

Your choice, as to which you use in your own code.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[nos]

Recommendation is to have a flag that tells the thread's
run() method to exit. Instead of interrupting the thread set the flag.
You can start a new thread on a new array of names.

 

[Chris Smith]

Recommendation is to have a flag that tells the thread's
run() method to exit. Instead of interrupting the thread set the flag.
You can start a new thread on a new array of names.

I'm not sure if I agree with that recommendation. Do you have a reason
to so recommend? After all, using interrupt() for that purpose seems to
be the intention of the API, and the effect it has on interruptible
operations like wait/sleep allow it to be used with much better effect
than polling. If you poll, then delay in ending a cancellable action
like this could be substantial, even permanent in the case of a waiting
thread.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[nos]

Beg to differ. My book says "interrupt() wakes up a waiting
or sleeping thread (with an interrupedException) or sets an
interrupted flag on a non sleeping thread. A thread can test
its own interrupted flag with the static interrupted() method."
So if it is running the best you can do is poll a flag anyway.
If it is sleeping or waiting it will have to check its interrupted
flag in the exception code. So the only thing you gain with a
interrupt() is to wake it up so it can kill itself.

Perhaps a more important issue is about the "open" status
of the file. I have a java program that looks at all jpg/gif
files in a directory and deletes the small ones. When I tried to
close and delete a file from the primary loop
the delete does not usually succeed because
the file is still considered open by windows even if it has
already been closed by java. I had to use "deleteOnExit()".

 

[Steve Horsley]

One further comment. You could also use Thread.join here to wait for
the thread to complete, instead of wait/notify and an isFinished method.

But since he is starting the thread from the GUI event dispatch thread,
calling join would possibly freeze the GUI for a while. I thkn using
notify() is the better approach.

Steve.

 

[Steve Horsley]

Beg to differ. My book says "interrupt() wakes up a waiting
or sleeping thread (with an interrupedException) or sets an
interrupted flag on a non sleeping thread. A thread can test
its own interrupted flag with the static interrupted() method."
So if it is running the best you can do is poll a flag anyway.
If it is sleeping or waiting it will have to check its interrupted
flag in the exception code.

It is not necessary to re-test inside the exception handler. You don't
get an InterruptedException thrown unless you have been interrupted.

So the only thing you gain with a
interrupt() is to wake it up so it can kill itself.

Yup. This could save a long pause during a blocked I/O call.

Steve.

 

[nos]

It's a disk read, how long is the delay 0.1 second?

 

[Chris Smith]

But since he is starting the thread from the GUI event dispatch thread,
calling join would possibly freeze the GUI for a while. I thkn using
notify() is the better approach.

Mmm, you're right. Unfortunately, wait() from the AWT event thread is
no better than join() from the AWT event thread. To solve that problem,
you'll need to use something like a callback with EventQueue.invokeLater
from the terminating thread to get a notification without waiting in a
way that blocks the thread.

If the thread itself shouldn't be modified to send the callback because
of separation of concerns in the application, one alternative is to
create a third utility thread that does the wait() or join() and then
does the EventQueue.invokeLater.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Chris Smith]

Beg to differ. My book says "interrupt() wakes up a waiting
or sleeping thread (with an interrupedException) or sets an
interrupted flag on a non sleeping thread. A thread can test
its own interrupted flag with the static interrupted() method."
So if it is running the best you can do is poll a flag anyway.
If it is sleeping or waiting it will have to check its interrupted
flag in the exception code. So the only thing you gain with a
interrupt() is to wake it up so it can kill itself.

Exactly. As Steve said, that's the point. If you want a thread to
terminate, then you may not want it to finish its current sleep or wait
call. That's the exact situation that interrupt() handles.

Perhaps a more important issue is about the "open" status
of the file.

Feel free to post a new message about this, if you haven't already.
It's not related at all to this discussion.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[contrex]

When should the thread read this flag?
Even if the thread would read the flag in its forNext loop this
wouldn't work:

for(int x=0; x<allThumbs.length && !stopFlag; x++){
allThumbs[x]=new MyThumb(images[x]);//CRASH
}

The stopFlag could be changed after the test "!stopFlag" and the
execution "MyThumb(images[index])". Thus the the thread would entry in
the next step of the loop. If the stopFlag (and the images array) now
changes then the call of images[x] would chrash.

 

[contrex]

Thank you for your answer! I see clearer now.
I tried to make what you suggested:

while (!updateTask.isFinished()) wait();

But there is an error:
java.lang.IllegalMonitorStateException: current thread not owner
at java.lang.Object.wait(Native Method)

What I have to do to become the owner of the thread in order to
execute wait() on it?

Thanks in advance!

 

[Chris Smith]

It's a disk read, how long is the delay 0.1 second?

Sorry? What's a disk read? There hasn't been a definite example posted
here; a thread could be blocked on a disk read, or it could be blocked
on waiting for a monitor notification, a timed sleep, or any number of
other things.

The point is, a claim was made that a roll-your-own "stop" flag is a
better idea than interrupt(). I don't think so, and I and Steve have
been explaining why interrupt() is better in many cases. On the other
hand, I've seen no one actually suggesting a problem with using
interrupt() for its intended purpose. Do you see a problem?

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Chris Smith]

Thank you for your answer! I see clearer now.
I tried to make what you suggested:

while (!updateTask.isFinished()) wait();

But there is an error:
java.lang.IllegalMonitorStateException: current thread not owner
at java.lang.Object.wait(Native Method)

What I have to do to become the owner of the thread in order to
execute wait() on it?

You want to become the owner of the monitor, not the owner of the
thread. To do so, you use a synchronized block on the object's monitor:

synchronized (obj)
{
...
}

The reference obj should point to the same object to which you send the
wait() method call.

Nevertheless, keep in mind that the code I provided is not production
code to stick into your app. I intended it mostly to point you in the
right direction in terms of API docs and other references you might
have. You'll need to pay attention and figure out which monitor is the
best one to use, and also ensure that the terminating thread sets the
flag and notifies the monitor.

Also, be sure to read the part of this thread where someone points out
that you shouldn't be doing any waiting at all in the AWT event thread,
and perhaps my suggestion of a third-party auxiliary thread along with
EventQueue.invokeLater to solve that problem.

Here's a semi-complete example of something along the lines of what you
want:

class LongTask implements Runnable
{
private static final int REALLY_LARGE_NUMBER = 2000000000;

private boolean finished = false;

public synchronized boolean isFinished()
{
return finished;
}

private synchronized void setFinished()
{
finished = true;
notify();
}

public void run()
{
try
{
for (int i = 0; i < REALLY_LARGE_NUMBER; i++)
{
if (Thread.interrupted())
{
throw new InterruptedException();
}

someTask();
}
}
catch (InterruptedException e) { }

setFinished();
}
}

and later,

public class MyListener implements ActionListener
{
private LongTask task = null;
private Thread thread = null;

private boolean pending = false;

public void actionPerformed(final ActionEvent e)
{
if (pending) return;

if (task != null)
{
final LongTask taskCopy = task;
thread.interrupt();

new Thread(new Runnable() {
public void run()
{
try
{
synchronized (taskCopy)
{
while (!taskCopy.isFinished)
{
taskCopy.wait();
}
}
EventQueue.invokeLater(new Runnable() {
public void run()
{
task = null;
thread = null;
pending = false;
actionPerformed(e);
}
});
}
catch (InterruptedException e) { }
}
}).start();

pending = true;
}
else
{
// proceed, knowing that the old thread has completed.
}
}
}

Yes, that looks pretty complicated, but there's a lot going on.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[nos]

Actually, I agree with you guys.

Sorry? What's a disk read? There hasn't been a definite example posted
here; a thread could be blocked on a disk read, or it could be blocked
on waiting for a monitor notification, a timed sleep, or any number of
other things.

The point is, a claim was made that a roll-your-own "stop" flag is a
better idea than interrupt(). I don't think so, and I and Steve have
been explaining why interrupt() is better in many cases. On the other
hand, I've seen no one actually suggesting a problem with using
interrupt() for its intended purpose. Do you see a problem?

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[nos]

The name of the file to process is being passed to
MyThumb. It is just as safe as any other program you
can write that reads a file. Sure the user can delete files
from another program or shell but in that case the
methods used by MyThumb will return an error or
exception which need to be handled in any case.
So no crash, sorry.

When should the thread read this flag?
Even if the thread would read the flag in its forNext loop this
wouldn't work:

for(int x=0; x<allThumbs.length && !stopFlag; x++){
allThumbs[x]=new MyThumb(images[x]);//CRASH
}

The stopFlag could be changed after the test "!stopFlag" and the
execution "MyThumb(images[index])". Thus the the thread would entry in
the next step of the loop. If the stopFlag (and the images array) now
changes then the call of images[x] would chrash.



"nos" <[email protected]> wrote in message

Recommendation is to have a flag that tells the thread's
run() method to exit. Instead of interrupting the thread set the flag.
You can start a new thread on a new array of names.

 

[Steve Horsley]

Can you guarantee it's not a network disk?

 

[nos]

no, i hope it is one of the new iSCSI SANs
rather than nfs
if it is nfs i give up, you win