Win a copy of Practical SVG this week in the HTML/CSS/JavaScript forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Guarded Objects & NotifyAll()

 
Frank Schawillie
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok I must have missed sometime very obvious. I�m trying to create a guarded Thread() that will only execute when the Thread() counter is less than set max value.

The first three threads fall into the execute state and the next three threads will go into a wait state. This is what I wanted; the issue is on completion of an executing thread, I expected that the NotifyAll() would wake one of the waiting threads but it�s not?

thanxs

public class GuardedObject extends Thread {

private String name;
private static final int maxNumberOfActiveThreads = 3;
private static int currentNumberOfActiveThreads = 0;

GuardedObject(String name) {
this.name = name;
}

public void run() {
boolean loopControl = true;
while (loopControl) {
try {
if (currentNumberOfActiveThreads < maxNumberOfActiveThreads) {
currentNumberOfActiveThreads++;
System.out.println(this.name + ": Running!!");
for (int i = 0; i < 1000; i++) {
System.out.println(this.name + " :" + i);
}
synchronized (this) {
currentNumberOfActiveThreads--;
loopControl = false;
notifyAll();
System.out.println(this.name + ": Current number of active threads are: " + currentNumberOfActiveThreads);
}
} else {
synchronized (this) {
System.out.println(this.name + ": Too many active Threads");
wait();
}
}
} catch (InterruptedException e) {
System.out.println(this.name + ": I'm going to try again!");
}
}
}

public static void main(String[] args) {
GuardedObject a = new GuardedObject("a");
GuardedObject b = new GuardedObject("b");
GuardedObject c = new GuardedObject("c");
GuardedObject d = new GuardedObject("d");
GuardedObject e = new GuardedObject("e");
GuardedObject f = new GuardedObject("f");
a.start();
b.start();
c.start();
d.start();
e.start();
f.start();
}
}
 
Alan Walker
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would say the problem is that notifyAll is being executed on a different object (a different instance of the GuardedObject class) from the objects on which wait calls are being executed. So if object "d" is waiting, via effectively a call to this.wait(), it needs a call to the notifyAll method of the same object, "d". A call to the notifyAll() method of object "a" will not do the trick.
 
Stan James
(instanceof Sidekick)
Ranch Hand
Posts: 8791
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I agree. Try adding another static variable, say a String called "lock". Synchronize on lock and then lock.notifyAll() when you're done. Since it's static, all instances will be synchronizing on the same object.

Going at another angle ... if your true objective is to never run more than three of something at a time, look into thread pooling with Executor in JDK 5. You set up an executor with three threads and put any number of commands into a queue. Each thread will run one command and then try to get another command from the queue. As an added advantage you set up and tear down only three threads, not six (or six thousand depending on your application). It's generally cheaper to create more commands and fewer threads.
[ December 30, 2004: Message edited by: Stan James ]
 
Frank Schawillie
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanxs for your suggestions! I have modified the code base on your comments and it not works great!

Here is the final version & again thanxs 4 the help..

public class GuardedObject extends Thread {

private String name;
private static final int maxNumberOfActiveThreads = 3;
private static int currentNumberOfActiveThreads = 0;
private static Object lock = new Object();

GuardedObject(String name) {
this.name = name;
}

public void run() {
boolean loopControl = true;
while (loopControl) {
try {
if (currentNumberOfActiveThreads < maxNumberOfActiveThreads) {
currentNumberOfActiveThreads++;
System.out.println(this.name + ": Running!!");
for (int i = 0; i < 1000; i++) {
System.out.println(this.name + " :" + i);
}
synchronized (lock) {
currentNumberOfActiveThreads--;
loopControl = false;
lock.notifyAll();
System.out.println(this.name + ": Current number of active threads are: " + currentNumberOfActiveThreads);
}
} else {
synchronized (lock) {
System.out.println(this.name + ": Too many active Threads");
lock.wait();
}
}
} catch (InterruptedException e) {
System.out.println(this.name + ": I'm going to try again!");
}
}

public static void main(String[] args) {
GuardedObject a = new GuardedObject("a");
GuardedObject b = new GuardedObject("b");
GuardedObject c = new GuardedObject("c");
GuardedObject d = new GuardedObject("d");
GuardedObject e = new GuardedObject("e");
GuardedObject f = new GuardedObject("f");
a.start();
b.start();
c.start();
d.start();
e.start();
f.start();
}
 
Henry Wong
author
Sheriff
Posts: 22542
109
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanxs for your suggestions! I have modified the code base on your comments and it not works great!


It may works great -- or not, but there are definitely problems... Take a look at this code snippet.



You are reading and changing the currentNumberOfActiveThreads variable, but you are not protecting it with the lock you created. The scope that you are using to grab the lock for the wait/notify method is too small. It either needs to be increased to encompass the changing variables -- or new locks needs to be created. (I would choose increasing the scope in this case)

Henry
 
Henry Wong
author
Sheriff
Posts: 22542
109
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Another point, there is no reason to use notifyAll(). The notify() method is good enough. An exiting thread only needs to wake up one thread to replace it.

Henry
 
I think I'll just lie down here for a second. And ponder this tiny ad:
the new thread boost feature brings a LOT of attention to your favorite threads
https://coderanch.com/t/674455/Thread-Boost-feature
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!