How to use wait() and notifyAll() in simple container object

[Bryan]

Hello all,

I have a simple container object that has a get and set method for a
variable it contains. Multiple threads will potentially be accessing
the container object to get and set the variable. Can anyone tell me
what's wrong with the code I have below? It's not working for me...

public class Container {

private boolean value = false;
private boolean available = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
while (available == false) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
available = false;
notifyAll();
return value;
}

public synchronized void set(boolean value) {
while (available == true) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
this.value = value;
available = true;
notifyAll();
}
}

I found most of the above code in a tutorial on the web --
http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
it's not working for me. Here's how I'm testing it:

I have a class that extends TimerTask and is called every 5 seconds to
randomize the value by calling the set method and passing it a random
boolean value obtained from Random.nextBoolean(). I have it set up to
print to the console each time it changes the value... it prints one
time but never prints again. Any suggestions as to why this isn't
working for me?

Thanks!

 

[hiwa]

Hello all,

I have a simple container object that has a get and set method for a
variable it contains. Multiple threads will potentially be accessing
the container object to get and set the variable. Can anyone tell me
what's wrong with the code I have below? It's not working for me...

public class Container {

private boolean value = false;
private boolean available = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
while (available == false) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
available = false;
notifyAll();
return value;
}

public synchronized void set(boolean value) {
while (available == true) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
this.value = value;
available = true;
notifyAll();
}
}

I found most of the above code in a tutorial on the web --
http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
it's not working for me. Here's how I'm testing it:

I have a class that extends TimerTask and is called every 5 seconds to
randomize the value by calling the set method and passing it a random
boolean value obtained from Random.nextBoolean(). I have it set up to
print to the console each time it changes the value... it prints one
time but never prints again. Any suggestions as to why this isn't
working for me?

Thanks!

it's not working for me

'It' is your code, not the tutorial code which is too simple to have
any problem.
Post a small demo code that is generally compilable, runnable and could
reproduce your problem. See:
http://homepage1.nifty.com/algafield/sscce.html and
http://www.yoda.arachsys.com/java/newsgroups.html

 

[hiwa]

I have a class that extends TimerTask and is called every 5 seconds to

You should use multiple threads accessing your single Container object.
Otherwise, you can't test the synchronized/wait/notify mechanism.

'Container' is a bad naming.

 

[kilian heckrodt]

Hello all,

I have a simple container object that has a get and set method for a
variable it contains. Multiple threads will potentially be accessing
the container object to get and set the variable. Can anyone tell me
what's wrong with the code I have below? It's not working for me...

public class Container {

private boolean value = false;
private boolean available = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
while (available == false) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
available = false;
notifyAll();
return value;
}

public synchronized void set(boolean value) {
while (available == true) {
try {
wait();
} catch (InterruptedException ex) { ex.printStackTrace(); }
}
this.value = value;
available = true;
notifyAll();
}
}

I found most of the above code in a tutorial on the web --
http://www.janeg.ca/scjp/threads/synchronized.html -- but like I said
it's not working for me. Here's how I'm testing it:

I have a class that extends TimerTask and is called every 5 seconds to
randomize the value by calling the set method and passing it a random
boolean value obtained from Random.nextBoolean(). I have it set up to
print to the console each time it changes the value... it prints one
time but never prints again. Any suggestions as to why this isn't
working for me?

Thanks!

Hmmm... the example at the website looks ok, but i wonder whether you
misunderstood its purpose, i.e. how the consumer-producer scenario works.

The idea is that you _cannot_ set ("produce") a new value _before_ the
old is consumed. That means to call set for a 2nd time and having an
effect, you need to call get() before, which consumes the value and lets
you set/produce another later.
The consumer-producer scenario is to avoid race conditions and to make
sure no information gets overwritten by accident (a value that is
replaced by another via a set() call before it was consumed by a get() call.
This is a typical scenario for a multithreaded buffer, where you want to
avoid that its content gets overwritten by incoming new data before it
was read, since that would mean data loss

 

[Bryan]

Ah yes... I did in fact misunderstand the purpose. Thanks for clearing
that up. I simply want to make sure that one thread doesn't try to
read at the same time another thread is writing. If so, I want the
read to come after the write. Otherwise, I'm not worried about data
loss.

Thanks!

 

[andrewmcdonagh]

Ah yes... I did in fact misunderstand the purpose. Thanks for clearing
that up. I simply want to make sure that one thread doesn't try to
read at the same time another thread is writing. If so, I want the
read to come after the write. Otherwise, I'm not worried about data
loss.

Thanks!

If thats all you want to do,then you have a lot less choices...

1) Don't use objects whose values can be changed - use multiple
'immutable' objects. So the first thread uses the first object, then
the second thread one uses a different object based upon the value that
correct for it. (sounds compilcated and it is in some ways, but you
get great performance.

2) - The one which sounds like the best for your case. - Just
synchronize your methods like you have, without all of the extra
gubbings.

e.g.

public class Container {

private boolean value = false;

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
return value;
}

public synchronized void set(boolean value) {
this.value = value;
}

}

 

[Lew]

public class Container {

private boolean value = false; // redundant assignment

public Container(boolean value) {
this.value = value;
}

public synchronized boolean get() {
return value;
}

public synchronized void set(boolean value) {
this.value = value;
}

}

Looks like a mutable, synchronized Boolean. Might want to consider a class
name like SynchronizedBoolean. Might want to rename the get()/set() methods,
either to follow JavaBean conventions or to mimic similar methods in
java.lang.Boolean.

- Lew

 

[John Ersatznom]

Looks like a mutable, synchronized Boolean. Might want to consider a
class name like SynchronizedBoolean. Might want to rename the
get()/set() methods, either to follow JavaBean conventions or to mimic
similar methods in java.lang.Boolean.

- Lew

Why not

public class Container<Foo> {
private Foo contents;
public Container (Foo initialContents) {
contents = initialContents;
}
public synchronized Foo getContents () {
return contents;
}
public synchronized setContents (Foo newContents) {
contents = newContents;
}
}

Container<Foo> fooHolder = new Container<Foo>(new Boolean(false));

It's reusable for holding any object now.

 

[John Ersatznom]

Why not

public class Container<Foo> {
private Foo contents;
public Container (Foo initialContents) {
contents = initialContents;
}
public synchronized Foo getContents () {
return contents;
}
public synchronized setContents (Foo newContents) {
contents = newContents;
}
}

Container<Foo> fooHolder = new Container<Foo>(new Boolean(false));

Meh, make that last

Container<Boolean> booleanHolder = new Container<Boolean>(new
Boolean(false));

 

[Bryan]

Thanks for the comments guys. Looks like I'll go with synchronizing
the get/set methods!

 

[castillo.bryan]

Meh, make that last

Container<Boolean> booleanHolder = new Container<Boolean>(new
Boolean(false));

Why not just use AtomicBoolean then?

 

[andrewmcdonagh]

Container<Boolean> booleanHolder = new Container<Boolean>(new
Boolean(false));

Its rare that you want to new a Boolean, its better to use the already
existing constant


Container<Boolean> booleanHolder = new Container<Boolean>(
Boolean.FALSE);

 

[Thomas Hawtin]

There are good reasons for making type variables look different from types.

Why not just use AtomicBoolean then?

Or even just a volatile boolean.

Adding the keyword 'synchronized' does not automatically make the code
thread-safe.

Tom Hawtin