Syncrhonization woes

[Rik Sweeney]

Hi,

I have some code to pull an object out of a pool. The pool is just a vector and it lives in a singleton. I've synchronized both the obtain and release methods. Here is the code:

    public synchronized void releaseObject(Object c)
    {
        pool.add(c);
        System.out.println("Releasing Object");
        notify();
    }
    
    public synchronized Object getObject()
    {
        if (pool.size() == 0)
        {
            System.out.println("Couldn't get Object, waiting");
            
            try
            {
                wait();
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
        }
        
        System.out.println("Obtaining Object");
        
        return (Object) pool.remove(0);
    }

In my main class I created 500 Threads which all try and get an object, wait a bit and then release it. Since there are only 50 in the pool, I would expect to see the following:

Obtaining Object (50 times)
Couldn't get Object, waiting
Releasing Object
Obtaining Object
etc...

The problem is though that I get the "Obtaining Object" printed out 50 times and then "Couldn't get Object, waiting" another 450 times.

I don't get why this happens as surely only one Thread can access the getObject at any one time.

Does anyone have any suggestions?

Thanks

Richard

[Henry Wong]

The problem is though that I get the "Obtaining Object" printed out 50 times and then "Couldn't get Object, waiting" another 450 times.

I don't get why this happens as surely only one Thread can access the getObject at any one time.

The wait() method releases the synchronization lock while it is waiting. This would make sense as it would not be possible for the releaseObject() method to be called if the lock were not free.

Henry

[Ernest Friedman-Hill]

With few exceptions, wait() should always be called in a loop: "while(x) wait()" rather than "if (x) wait()". The reason is that a Thread may be notified, but then before wait() returns, some other thread's wait() also returns and invalidates the condition. The "if(x) wait()" assumes that when wait() returns, x has become false, but it ain't necessarily so. The "while" version will not make this mistake.

[Rik Sweeney]

Originally posted by Ernest Friedman-Hill:
With few exceptions, wait() should always be called in a loop: "while(x) wait()" rather than "if (x) wait()". The reason is that a Thread may be notified, but then before wait() returns, some other thread's wait() also returns and invalidates the condition. The "if(x) wait()" assumes that when wait() returns, x has become false, but it ain't necessarily so. The "while" version will not make this mistake.

OK,

so changing the code to

    
    public synchronized Object getObject()
    {
        while (pool.size() == 0)
        {
            System.out.println("Couldn't get Object, waiting");
            
            try
            {
                wait();
            }
            catch (Exception e)
            {
                e.printStackTrace();
            }
        }
        
        System.out.println("Obtaining Object");
        
        return (Object) pool.remove(0);
    }

Appears to have solved the problem, but please feel free to shout me down if this still isn't right!

Richard

[Ernest Friedman-Hill]

No, that's exactly what I meant!