Threads not communicating

Question

I am making a program in which a pool of x number of threads interacts with a shared inventory. In this case I use an ArrayList as the shared inventory. In my program the threads are representations of jobs that a creature has. Creatures belong to a party and share a pool of Artifacts used to perform jobs. only one creature my interact with the pool at any time.

For some reason I keep having issues with thread communication. I have set it up so if the pool is not being used the creature drops all its items into the pool and begins to look through it. And if they cannot find everything they need they should drop all the artifacts they have into the pool, Set the pool to to not busy, then signal one creature waiting on the pool that it is ready for them to go through it as well.

My issue is that if a creature is looking through the pool while another is waiting, and it cannot find what it like it repeats the process without notifying or letting the other creature go through it.

What I've attempted: I initially attempted using locks but the locks would not signal other threads. Then I rewrote it taking advantage of synchronized ( Party ). Then I thought it was because the threads were crashing somewhere but a thread can run all the way through and even dump its items back into the pool when the job is complete (given a creature has not locked the pool into limbo).

boolean ready = target.hasReqArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
    //checks to see if creature already has correct amount of each item.
    //If it does it should skip pool interaction until it dumps its used items
    //back into the pool.
    System.out.println( "Ready: " + ready );
    while ( !ready ) {//begin pool interaction
        synchronized ( target.poolParty ){
            System.out.println( "Ready: " + ready );
            System.out.println( this );
            while ( target.poolParty.busyPool ) {//busyPool is initialized false
                startJob.setEnabled( false );
                try {
                    target.poolParty.wait();
                } catch ( InterruptedException e ) {}
            }
            synchronized ( target.poolParty ) {
                target.poolParty.busyPool = true;
                target.poolParty.notifyAll();//notify all threads that they need to wait because this one will proceed
            }
        }
        target.releaseArtifacts();// adds all artifacts held by creature to an arraylist in poolParty
                                  //then clears the creatures inventory
        target.pickUpArtifacts( reqStones, reqPotions, reqWands, reqWeapons );

        ready = target.hasReqArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
        if ( ready ) {
            synchronized ( target.poolParty ) {
                System.out.println( "has required Items" );
                target.poolParty.busyPool = false;
                target.poolParty.notify();
            }
        } else {
            synchronized ( target.poolParty ) {
                System.out.println( "Does not has required Items" );
                target.poolParty.busyPool = false;
                target.releaseArtifacts();
                target.poolParty.notifyAll();
                try {
                    Thread.sleep( 1000 );
                } catch ( InterruptedException e ){}
            }
        }
    }//end pool interaction

I perform this kind of interaction between threads in a scenario where a creature has more than one job but can only do one job at a time and it works perfectly. I tailored those methods to match this situation and I am still having issues.

Note: I'm very new to concurrency so forgive me if my vocabulary is not as developed on the subject.

Answer 1

It looks like you are sleeping in a block that uses target.poolParty for synchronization. That means that the other threads waiting to access the pool won't be able to access it because this thread is blocking it. Move the sleep() outside that block.

At another place you use synchronized (target.poolParty) in a block that already uses that for synchronization. That is unnecessary (well, the whole block of code is unnecessary and I removed it. Just pointing out). Also wait() and notify*() are low level synchronization primitives and rarely needed.

boolean ready = target.hasReqArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
//checks to see if creature already has correct amount of each item.
//If it does it should skip pool interaction until it dumps its used items
//back into the pool.
System.out.println( "Ready: " + ready );
while ( !ready ) {
    // begin pool interaction
    synchronized (target.poolParty) {
        target.pickUpArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
        ready = target.hasReqArtifacts( reqStones, reqPotions, reqWands, reqWeapons );

        /* I'd move this here. If we never release out items then the
           other party members can't use them while this one still does
           not have the needed items. Now they will be available to
           other parties while this thread sleeps. */
        if (!ready) {
            // adds all artifacts held by creature to an arraylist in poolParty
            target.releaseArtifacts();
        }
    }

    // These don't access the pool, so they can be outside the synchronized block
    if ( ready ) {
        System.out.println( "has required Items" );
    } else {
        System.out.println( "Does not have required Items" );
        // Sleep before the next try to get the required items. Lets other
        // threads attempt to fetch their needed items
        try {
            Thread.sleep( 1000 );
        } catch ( InterruptedException e ) {
            // Added, because silently eating exceptions is a bad idea
            e.printStackTrace();
        }
    }
}//end pool interaction

Answer 2

Looks like you are over thinking the problem. Your use case is simple.

• Only one creature should be able to access the pool at any given time.
• Other creatures must wait during that time.
• Shared resource here is the pool.

Now a simple

synchronize(pool){
//doStuff
}

should work.

Note that wait() and notify()/notifyAll() are needed when you need to synchronize different portions of code which don't nicely fall into a single consecutive logic block.

boolean ready = target.hasReqArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
//checks to see if creature already has correct amount of each item.
//If it does it should skip pool interaction until it dumps its used items
//back into the pool.
System.out.println( "Ready: " + ready );
if ( !ready ) {//begin pool interaction
    synchronized ( target.poolParty ){
        System.out.println( "Ready: " + ready );
        System.out.println( this );
        startJob.setEnabled( false );
    }
    target.releaseArtifacts();// adds all artifacts held by creature to an arraylist in poolParty
                              //then clears the creatures inventory
    target.pickUpArtifacts( reqStones, reqPotions, reqWands, reqWeapons );
    }
}//end pool interaction