thread wait

K

korcs

Hi All,

I have a problem with suspending a thread.

As suspend() has deprecated I am trying to use the wait() - notify()
combination, but after the thread pauses it won't resume again.

Here is my code:

The two button listeners that should proceed the wait and notify
actions:

/**
* Start button listener
*/
class startListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation started\n");
threadsSuspended = false;

if(!clock.isAlive()) {

clock.start();

}
else {

synchronized(this) {
notifyAll();

}
}


} // method actionPerformed

} // class startListener


/**
* Stop button listener
*/
class stopListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation stopped\n");

threadsSuspended = true;


} // method actionPerformed

} // class stopListener

The run method

public void run()
{
try {


while(true)
{
this.putText("F\n");

this.counter++;
this.putText(new Integer(counter).toString());
Thread.sleep(clock_period);
synchronized(this) {
while (threadsSuspended)
wait();
}

} // while

} catch (InterruptedException e) {

}

return;

} // method run

I have a boolean threadsSuspended = true;

variable at the initialization of the class.

The thread is initialized by:

Thread clock = new Thread(this);

Does anyone have a clue why the code is not working?



Thanks for your help!

Best,

korcs
 
P

Patricia Shanahan

korcs said:
Hi All,

I have a problem with suspending a thread.

As suspend() has deprecated I am trying to use the wait() - notify()
combination, but after the thread pauses it won't resume again.
.....

Does anyone have a clue why the code is not working?

I can't work out what is going on, but one obvious problem is that it
looks as though some actions on threadsSuspended appear to be outside
the synchronized blocks.

Given the lack of a complete program, it is hard to check issues such as
whether the event handling thread gets suspended and whether the same
object is used for all synchronization that protects the same variable.

Any chance of an SSCCE? See http://homepage1.nifty.com/algafield/sscce.html

Patricia
 
T

Thomas Hawtin

korcs said:
I have a problem with suspending a thread.

It's really difficult to see what is happening with an incomplete
example. As Patricia mentioned, try to come up with a small but complete
program to demonstrate the problem.

Anyway, my thoughts on what you have selected:
As suspend() has deprecated I am trying to use the wait() - notify()
combination, but after the thread pauses it won't resume again.

Yes, look at the explanation. suspend can pause a thread whilst it is
holding a lock that you may not even be aware (perhaps in some library,
perhaps loading classes).
class startListener implements ActionListener {

Class names should have initial caps.

Tabs work badly. Particularly in Usenet. My reader is displaying you
post indented inconsistently. Quoting doesn't help either.
if(!clock.isAlive()) {

clock.start();

So clock is a thread. You can only start threads once. isAlive isn't a
good check. There is also a potential race from between isAlive, start
and the thread actually starting.

threadsSuspended = true;

Technically this should be in a synchronised block matching reading of
the variable (you can use volatile, but that is very difficult to get
right).
public void run()
{

I'd suggest using an inner class, even if it just calls a private method
on the outer class.

Note that 'this' within an inner class refers to inner instance, not the
outer. Even Brian Goetz managed to publish a book with this mistake.
this.putText(new Integer(counter).toString());

I don't know the implementation of putText, but note that Swing
components should always be accessed on the AWT Event Dispatch Thread (EDT).

Integer.toString(counter) would be a better way of expressing that (or
String.valueOf(counter) if you must).
Thread.sleep(clock_period);
synchronized(this) {
while (threadsSuspended)
wait();
}

Are you sleeping or waiting? (Unless it's a bug demonstration, I always
prefer timed wait to sleep as you can quite a wait without having to use
interrupt (which has some problems associated with it)).
} // while

} catch (InterruptedException e) {

}

Yay! Thread interrupts that actually interrupts rather than being
dropped and carrying on as normal.

Unnecessary.

Tom Hawtin
 
K

korcs

I can't work out what is going on, but one obvious problem is that it
looks as though some actions on threadsSuspended appear to be outside
the synchronized blocks.

Given the lack of a complete program, it is hard to check issues such as
whether the event handling thread gets suspended and whether the same
object is used for all synchronization that protects the same variable.

Any chance of an SSCCE? Seehttp://homepage1.nifty.com/algafield/sscce.html

Patricia



Hi Patricia,

Here is an SSCCE:

import java.awt.*;
import java.awt.event.*;
import java.util.Vector;



public class DynamicPathGenerator extends Frame implements Runnable {


/**
* Member variables
*
* @param clock Thread that provides the ticks for the main program
* @param clock_period Shows how often the clock ticks
* @param serialVersionUID Id needed for serialization
* @param ta TextArea for system outputs
*/
Thread clock;
boolean threadsSuspended = false;
static final int clock_period = 1000;
private static final long serialVersionUID = 1L;
TextArea ta = new TextArea("DynamicPathGenerator ver 1.0\n\n", 10,
35);
int counter;

/**
* AWT classes
*/

/**
* Window closing class
*/
class MyWindowCloser extends WindowAdapter {

public void windowClosing(WindowEvent e) {

System.exit(0);

} // method windowClosing

} // class MyWindowCloser

/**
* Start button listener
*/
class startListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation started\n");
threadsSuspended = false;

if(!clock.isAlive()) {

clock.start();

}
else {

synchronized(this) {
notifyAll();

}
}


} // method actionPerformed

} // class startListener


/**
* Stop button listener
*/
class stopListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation stopped\n");

threadsSuspended = true;


} // method actionPerformed

} // class stopListener


/**
* Constructor of the class.
*/
public DynamicPathGenerator() { // CONSTRUCTOR

clock = new Thread(this);

Button start = new Button("Simulation starten"); //Gombok
hozzáadása
Button stop = new Button("Simulation beenden");

Panel p1 = new Panel(new GridLayout(2,3));
Panel p3 = new Panel();

// add panel elements

p1.add(start);
p1.add(stop);
p3.add(ta);

// arrange panel elements

add(p1, BorderLayout.NORTH); add(p3, BorderLayout.SOUTH);

// adding listeners

start.addActionListener(new startListener () );
stop.addActionListener(new stopListener () );

addWindowListener( new MyWindowCloser () );


} // CONSTRUCTOR



public void putText(String text) {
ta.append(text);
}

public void run()
{
try {


while(true)
{
this.putText("F\n");

this.counter++;
this.putText(new Integer(counter).toString());
Thread.sleep(clock_period);
synchronized(this) {
while (threadsSuspended)
wait();
}

} // while

} catch (InterruptedException e) {

}

return;

} // method run






/**
* The main method
*/
public static void main(String argv[]) throws Exception {

DynamicPathGenerator dpg = new DynamicPathGenerator();
dpg.pack();
dpg.setVisible(true);

} // method main

} // class DynamicPathGenerator

I am really quite newby with programming threads, so I think I still
have some basic problems of understanding the concepts of thread
programming.

I read the official sun docs on the issue and I used the codes
there... So thanks if someone can point out the bug in the code above.

Best,

korcs
 
L

Lew

korcs wrote:
Here is an SSCCE:

import java.awt.*;
import java.awt.event.*;
import java.util.Vector;

You might find ArrayList preferable.
public class DynamicPathGenerator extends Frame implements Runnable {

It's a little unusual to launch an entire Frame (why not a JFrame?) in a
subthread of itself. In your case that is probably not the thing to do.

Don't use TAB characters in Usenet posts.
* Member variables
*
* @param clock Thread that provides the ticks for the main program
* @param clock_period Shows how often the clock ticks
* @param serialVersionUID Id needed for serialization
* @param ta TextArea for system outputs
*/
Thread clock;
boolean threadsSuspended = false;

Redundant initialization.

You should consider providing access specification for your instance
variables, almost always "private".
static final int clock_period = 1000;

Compile time constants conventionally are named in all uppercase letters.
Private? Public?

Code is a little easier to read if you put all static variables together and
all non-static variables together in separate "paragraphs".
private static final long serialVersionUID = 1L;
TextArea ta = new TextArea("DynamicPathGenerator ver 1.0\n\n", 10,
35);
int counter;

/**
* AWT classes
*/

/**
* Window closing class
*/
class MyWindowCloser extends WindowAdapter {

public void windowClosing(WindowEvent e) {

System.exit(0);

} // method windowClosing

} // class MyWindowCloser

/**
* Start button listener
*/
class startListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation started\n");
threadsSuspended = false;

if(!clock.isAlive()) {

clock.start();

}
else {

synchronized(this) {

Remember what Thomas Hawtin said, echoing Patricia Shanahan's advice?
notifyAll();
}
}


} // method actionPerformed

} // class startListener


/**
* Stop button listener
*/
class stopListener implements ActionListener {

public void actionPerformed (ActionEvent e) {

ta.append("DynamicPathGenerator> simulation stopped\n");

threadsSuspended = true;


} // method actionPerformed

} // class stopListener


/**
* Constructor of the class.
*/
public DynamicPathGenerator() { // CONSTRUCTOR

clock = new Thread(this);

Button start = new Button("Simulation starten"); //Gombok
hozzáadása
Button stop = new Button("Simulation beenden");

Panel p1 = new Panel(new GridLayout(2,3));
Panel p3 = new Panel();

// add panel elements

p1.add(start);
p1.add(stop);
p3.add(ta);

// arrange panel elements

add(p1, BorderLayout.NORTH); add(p3, BorderLayout.SOUTH);

// adding listeners

start.addActionListener(new startListener () );
stop.addActionListener(new stopListener () );

addWindowListener( new MyWindowCloser () );
} // CONSTRUCTOR



public void putText(String text) {
ta.append(text);
}

public void run()
{

Why do you need to run a whole Frame in the subthread?
try {
while(true)
{
this.putText("F\n");

this.counter++;
this.putText(new Integer(counter).toString());

You ignored the advice about Integer.toString(counter), I see.
Thread.sleep(clock_period);
synchronized(this) {

wait() and notify() / notifyAll() have to be called on the same monitor.
while (threadsSuspended)
wait();
}
} // while
} catch (InterruptedException e) {
}

return;

You ignored the advice about the redundant "return", I see.
} // method run
/**
* The main method
*/
public static void main(String argv[]) throws Exception {

Why are you throwing an Exception from main()?
DynamicPathGenerator dpg = new DynamicPathGenerator();
dpg.pack();
dpg.setVisible(true);

} // method main

} // class DynamicPathGenerator


HTH.
 
P

Patricia Shanahan

Lew said:
korcs wrote: ....
public static void main(String argv[]) throws Exception {

Why are you throwing an Exception from main()?

I often throw exceptions from main in small test and example programs.
It minimizes the amount of exception processing code that is needed
inside the program, without hiding the exceptions.

Patricia
 
P

Patricia Shanahan

korcs wrote:
....
I am really quite newby with programming threads, so I think I still
have some basic problems of understanding the concepts of thread
programming.
....

There is one key concept you are missing. All the wait and notify
actions that you intend to interact MUST use the same object.

You use "this", both implicitly, "wait;", and explicitly,
"synchronized(this)". That does not work, because which object is meant
by "this" depends on where the code is. "this" inside your
ActionListener objects is not the same Object as "this" inside your run
method.

I would declare a threadsSuspensionLock Object right next to the
threadsSuspended declaration. Make ALL synchronization, notify, and wait
actions that relate to threadsSuspended use that object.

synchronized(threadsSuspensionLock)

threadsSuspensionLock.wait()

threadsSuspensionLock.notify()

Obviously, pick a good identifier according to your coding conventions.

Patricia
 
K

korcs

You use "this", both implicitly, "wait;", and explicitly,
"synchronized(this)". That does not work, because which object is meant
by "this" depends on where the code is. "this" inside your
ActionListener objects is not the same Object as "this" inside your run
method.

I would declare a threadsSuspensionLock Object right next to the
threadsSuspended declaration. Make ALL synchronization, notify, and wait
actions that relate to threadsSuspended use that object.

synchronized(threadsSuspensionLock)

threadsSuspensionLock.wait()

threadsSuspensionLock.notify()

Obviously, pick a good identifier according to your coding conventions.

Patricia

Hey, Patricia thanks a lot it works perfectly!

For the other contributors thanks of course as well.

Best,

korcs
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top