CODEWITHSUNDEEP
javamultithreadingconcurrencycounter
Sam Castledine
Sam Castledine

Reputation: 105

Concurrent download counters in Java

As part of our University coursework we have to make a multi threading download server in Java. Everything is running smoothly apart from one bit : we have to have the server show the total number of downloads for each item each time it is downloaded. So far I have gotten it to work unless both clients request it at the same time. The code is below, If any one has any ides I would be very grateful. Also we must include thread.sleep part and must increment the counter in that convoluted way.

//Snipper from Protocol.java

if (theInput.equals("1")) {

            theOutput = "The program displays a message... Another? Y or N";


            DownloadCounter counter = new DownloadCounter();

            count = DownloadCounter.getcount();//count is a var in Protocol.java it is                      static

            int tmp = count;
            try {
                Thread.sleep(5000);
            } catch (InterruptedException ex) {
                System.out.println("sleep interrupted");
            }
            count = tmp + 1;

            DownloadCounter.setcount(count);

            System.out.println("Download Total " + count);


            state = ANOTHER;

The DownloadCounter: 

//DownloadCounter.java
public class DownloadCounter {

    private static int count;

    public static synchronized int getcount(){
        return count;
    }

    public static synchronized void setcount(int num){
        DownloadCounter.count = num;
    }
}

Upvotes: 2

Views: 1384

Answers (3)

Kiril
Kiril

Reputation: 40365

The fundamental problem is that you have two threads doing a get, increment and set, so consider this situation:

Thread 1: set(5) // now count is 5
Thread 1: get() // Thread 1 gets 5
Thread 2: get() // Thread 2 gets 5
Thread 2: increments its local copy of count to 6
Thread 1: increments its local copy of count to 6
Thread 2: set(6) // now the count is 6
Thread 1: set(6) // the count is still 6, but it should be 7!!!

The solution is to implement an increment method which increments the count in a thread safe manner:

public synchronized void increment()
{
    count++;
}

You can also use an AtomicInteger and avoid the locking:

AtomicInteger count = new AtomicInteger(0);

public int getCount()
{
    return count.get();
}

public void increment()
{
    count.incrementAndGet();
}

You also stated that the counter should count the number of downloads for each item, however, your current code will not do that. Your current counter will count ALL of the downloads for ALL of the items. Hint: you're making everything in DownloadCounter static and that's not going to work well if you want to have a separate counter for each item.

Upvotes: 4

JB Nizet
JB Nizet

Reputation: 691865

The key to make it correct is precisely to make the get/increment/set an atomic operation. Instead of the setCount method, there should be a synchronized incrementCount() method.

You could also avoid the synchronization completely by using an AtomicInteger and use its incrementAndGet() method inside the incrementCount() method.

Note that the instruction DownloadCounter counter = new DownloadCounter(); is completely unnecessary. The class should have a private constructor to prevent such unnecessary instantiations.

Upvotes: 0

Michael Krussel
Michael Krussel

Reputation: 2656

DownloadCounter needs a method for incrementing. There's no safe way to increment the counter with only a getCount and setCount method.

Java has a class AtomicInteger for handling just this type of thing.

Also you are only calling static methods on DownloadCounter, so there is no need to create a new instance.

Upvotes: 0

Related Questions