CODEWITHSUNDEEP
javamultithreading
Asparatame
Asparatame

Reputation: 3

Why is my threaded output showing the same message twice?

So I'm making a program for a class. The objective of the program is to have a few persons eat fruits from a bag. The fruits give energy and the energy of the persons are decreasing every second. The problem I'm having is that every time a fruit is eaten I'm doing an output which looks like this:

System.out.println("There are now " + fruitList.size() + " fruits left in the bag.");

This should, as far as I understand, output the current size of my arraylist. What I'm confused about is that if two people (initialized through two different threads) are eating at about the same time, this should show for example first, "There are now 10 fruits left in the bag.", and then "There are now 9 fruits left in the bag.". The problem is that both of those messages will show that there are 10 fruits left.

This is the method in class Person that is responsible for this action:

public void eat(Fruit fruit, ArrayList<Fruit> fruitList, MagicBag magicBag) {
    lock.lock();
    try {
        while (getEnergy() > 20) {
            newDeposit.await();
        }
        if(energy < 30 && energy > 20) {
            System.out.println(this.getName() + " is getting hungry.");
        } else {
        System.out.println(this.getName() + " picked a " + fruitList.get(fruitList.size() - 1).getName() + " and started to eat...");
        Thread.sleep(1000);
        int fruitEnergy;
        fruitEnergy = fruitList.get(fruitList.size() - 1).getEnergyValue();
        fruitList.remove(fruitList.size() - 1);
        if(energy > 100) {
            energy = 100;
        } else {
            energy += fruitEnergy;
        }
        System.out.println(this.getName() + " ate a " + fruitList.get(fruitList.size() - 1).getName() + "!");
        System.out.println("There are now " + fruitList.size() + " fruits left in the bag.");
        magicBag.addFruitsToBag(fruitList, fruit);
        newDeposit.signalAll();
        }
        } catch (InterruptedException e) {
            e.printStackTrace();
        } finally {
            lock.unlock();
        }
}

And this is what run() is doing in the class that implements Runnable which is running the above code:

public void run() {
    // TODO Auto-generated method stub
    try {
        while(running) {
            if(fruitList.size() > 0) {
            person.eat(fruit, fruitList, magicBag);
            Thread.sleep(1000);
            }
            else {
                kill();
            }
        }
    } catch(InterruptedException e) {
        e.printStackTrace();
    } catch(IndexOutOfBoundsException ie) {
        running = false;
    } catch(NullPointerException ne) {
        ne.printStackTrace();
    }
}

So if anyone could help me that would be awesome. If you need anything else just ask.

Upvotes: 0

Views: 76

Answers (1)

chrylis -cautiouslyoptimistic-
chrylis -cautiouslyoptimistic-

Reputation: 77177

You have no memory barriers on that fruitList, so your threads are simultaneously inspecting and modifying the same object. You need to either synchronize on fruitList (preferable in this case, since that's the resource being shared) or use some other mechanism to ensure that there's a memory barrier, such as by using a concurrent collection.

Upvotes: 3

Related Questions