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

Discussion in 'Java' started by Bryan, Dec 15, 2006.

  1. Bryan

    Bryan Guest

    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!
    Bryan, Dec 15, 2006
    #1
    1. Advertising

  2. Bryan

    hiwa Guest

    Bryan wrote:
    > 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, Dec 15, 2006
    #2
    1. Advertising

  3. Bryan

    hiwa Guest

    Bryan wrote:

    > 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.
    hiwa, Dec 16, 2006
    #3
  4. Bryan wrote:
    > 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
    kilian heckrodt, Dec 16, 2006
    #4
  5. Bryan

    Bryan Guest

    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!

    > 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, Dec 17, 2006
    #5
  6. On Dec 17, 4:51 pm, "Bryan" <> wrote:
    > 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!
    >
    > > 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



    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;
    }

    }
    andrewmcdonagh, Dec 17, 2006
    #6
  7. Bryan

    Lew Guest

    > 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
    Lew, Dec 17, 2006
    #7
  8. Lew wrote:
    >> 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


    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, Dec 17, 2006
    #8
  9. John Ersatznom wrote:
    > Lew wrote:
    >
    >>> 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

    >
    >
    > 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));
    John Ersatznom, Dec 17, 2006
    #9
  10. Bryan

    Bryan Guest

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

    On Dec 17, 12:44 pm, John Ersatznom <> wrote:
    > John Ersatznom wrote:
    > > Lew wrote:

    >
    > >>> 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

    >
    > > 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, Dec 19, 2006
    #10
  11. Bryan

    Guest

    John Ersatznom wrote:
    > John Ersatznom wrote:
    > > Lew wrote:
    > >
    > >>> 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

    > >
    > >
    > > 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));



    Why not just use AtomicBoolean then?
    , Dec 19, 2006
    #11
  12. On Dec 17, 7:44 pm, John Ersatznom <> wrote:
    > John Ersatznom wrote:
    > > Lew wrote:

    >
    > >>> 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

    >
    > > 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));


    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);
    andrewmcdonagh, Dec 19, 2006
    #12
  13. wrote:
    > John Ersatznom wrote:
    >> John Ersatznom wrote:


    >>> public class Container<Foo> {


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

    >> Meh, make that last
    >>
    >> Container<Boolean> booleanHolder = new Container<Boolean>(new
    >> Boolean(false));


    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
    Thomas Hawtin, Dec 19, 2006
    #13
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. lonelyplanet999
    Replies:
    1
    Views:
    494
    A. Bolmarcich
    Nov 18, 2003
  2. Vera
    Replies:
    5
    Views:
    847
  3. mallikk
    Replies:
    0
    Views:
    633
    mallikk
    Oct 24, 2007
  4. Danny
    Replies:
    18
    Views:
    813
    Daniel Pitts
    Dec 15, 2008
  5. cuffJ
    Replies:
    0
    Views:
    303
    cuffJ
    Aug 16, 2010
Loading...

Share This Page