Timer Deadlock Issue

Question

I'm trying to set up a timer thread that once a second increments a counter and outputs the result to the terminal.

public class SecondCounter implements Runnable
{
private volatile int counter;
private boolean active;
private Thread thread;

public SecondCounter()
{
    counter = 0;
    active = true;
    thread = new Thread(this);
    thread.start();
}

public int getCount()
{
    return counter;
}

public void run()
{
    while(active)
    {
        try {
            Thread.sleep(1000);
        } catch(InterruptedException ex) {ex.printStackTrace();}

        synchronized(this)
        {
            System.out.print(++counter+" ");
            try{
                notifyAll();
                wait();
            } catch(InterruptedException ex) {ex.printStackTrace();}
        }
    }
}

Then, I have another method in the class called messagePrinter() that takes in an integer, creates a new thread, and monitors the main timer thread to see when a multiple of that int is on the count:

public synchronized void messagePrinter(final int x)
{
    Runnable mp = new Runnable()
    {
        public void run()
        {
            while(active)
            {
                synchronized(this)
                {
                    try {
                        while(counter%x != 0 || counter == 0 )
                        {
                            notifyAll();
                            wait();
                        }
                        System.out.println("\n"+x+" second message");
                        notifyAll();
                    } catch(InterruptedException ex) {ex.printStackTrace();}
                }
            }
        }
    };
    new Thread(mp).start();
}

I have tried messing around with wait() and notifyAll() quite a bit but every combination I have tried results in both threads entering a wait state and causing deadlock. Or, the timer thread will hog all of the thread time and never give the messagePrinter a chance to check what the count is currently at.

Here is what the output should look like:

1 2 3
3 second message
4 5 6
3 second message

I am aware that the timer is probably not going to time to be perfectly 1 second per tick with this method, but the point of the exercise was to get some experience with passing information between threads.

Here is my main file:

public class Main2
{
    public static void main(String[] args)
    {
        SecondCounter c = new SecondCounter();
        c.messagePrinter(3);
    }
}

Anyone with some threading experience care to give me some insight on where I am going wrong?

EDIT: I have converted my integer counter to an atomic integer, and I changed the messagePrinter to be synchronized on "SecondCounter.this" instead of "this". It's working now! Mostly anyway, it's printing "x second message" about thirty times a loop when there are multiple messagePrinters. I think I can fix that though.

Your `synchronized` section are synchronized by different `this` objects. First one uses instance of `SecondCounter`, second uses instance of your created `Runnable`. I'm pretty new to java, but I think you should use `synchronized (SecondCounter.this) ` in the second case, (and `SecondCounter.wait` / `SecondCounter.notifyAll`). — Dmitry Ermolov, May 03 '16 at 23:41
@dim-an Your suggestion seemed to do the trick! Although, I needed to call `SecondCounter.this.wait`, etc. — Stalemate Of Tuning, May 04 '16 at 00:05

score 0 · Accepted Answer · answered May 04 '16 at 00:10

I have it working now, here is my working code:

import java.util.concurrent.atomic.AtomicInteger;

public class SecondCounter implements Runnable
{
private volatile AtomicInteger counter;
private boolean active;
private boolean printed;
private Thread thread;

public SecondCounter()
{
    counter = new AtomicInteger(0);
    active = true;
    thread = new Thread(this);
    thread.start();
}



public void run()
{

    while(active)
    {
        try {
            Thread.sleep(1000);
        } catch(InterruptedException ex) {ex.printStackTrace();}

        synchronized(this)
        {
            System.out.print(counter.incrementAndGet()+" ");    
            printed = false;
            try{
                this.notify();
                this.wait();
            } catch(InterruptedException ex) {ex.printStackTrace();}
        }
    }
}

public synchronized void messagePrinter(final int x)
{
    Runnable mp = new Runnable()
    {
        public void run()
        {
            while(active)
            {
                synchronized(SecondCounter.this)
                {
                    try {
                        while(counter.get()%x != 0 || counter.get() == 0 )
                        {
                            SecondCounter.this.notify();
                            SecondCounter.this.wait();
                        }
                        if(!printed)
                        {
                            System.out.println("\n"+x+" second message");
                            printed = true;
                        }
                        SecondCounter.this.notify();
                        SecondCounter.this.wait();
                    } catch(InterruptedException ex) {ex.printStackTrace();}
                }
            }
        }
    };
    new Thread(mp).start();
}

}

Timer Deadlock Issue

1 Answers1