Help with SYNCHRONIZED

Hi

Starting to learn about Threads and locking shared resources but I have tried this out but it doesnt seem to be working, im sure I have missed something out somewhere but cannot find it :confused:

The purpose of this program is to allow exclusive access to the shared resource for one thread at a time so the results are not duplicated e.g. Thread 1 = 1, Thread 2 = 2 etc

I seem to be getting Threads with the same number (shared resource) so it obviously isnt locking the resource properly. If any one can point me in the right direction that would be great.


Shared Class:

Code :

---

package threads;
 
public class Shared
{
    private static int num = 0;
 
 
    public synchronized static void addToNum()
    {
        num++;
    }
 
    public synchronized static int getNum()
    {
        return num;
    }
 
}

---

Threads Class:

Code :

---

package threads;
 
public class Threads extends Thread
{
    private String Tname;
 
    public Threads(String name)
    {
        Tname = name;
    }
 
 
    public void process()
    {
         Shared.addToNum();
         System.out.println("Thread: " + Tname + " is number: " + Shared.getNum());
    }
 
 
    public void run()
    {
         process();
    }
 
}

---

Main Class:

Code :

---

package threads;
 
public class StartThreadsMain
{
 
    public static void main(String[] args)
    {
        Threads t1 = new Threads("T1");
        Threads t2 = new Threads("T2");
        Threads t3 = new Threads("T3");
        Threads t4 = new Threads("T4");
 
        t1.start();
        t2.start();
        t3.start();
        t4.start();
 
    }
 
}

---

Incorrect Output:

Code :

---

run:
Thread: T3 is number: 3
Thread: T2 is number: 4
Thread: T4 is number: 4
Thread: T1 is number: 3
BUILD SUCCESSFUL (total time: 0 seconds)

---

Correct Output:

Code :

---

run:
Thread: T1 is number: 1
Thread: T2 is number: 2
Thread: T3 is number: 3
Thread: T4 is number: 4
BUILD SUCCESSFUL (total time: 0 seconds)

---

Please post the output from the program that shows the problem.

What would be the results if all the threads called addTonum() and then they all called getNum?

Hi

I have now added the output to the original post.

It theory if all the threads called addToNum() then they call called getNum() the result would be 4 due to the 4 threads calling addToNum() so incrementing this to 4 then they would all call getNum() would result in 4 due to it being a shared resource.

I understand that the threads would not necessarily complete in order but I shouldn't be getting the same result for different threads which I am.

Quote:

---

I shouldn't be getting the same result for different threads

---

Why not? Please explain why you think that.
The OS determines when each of the threads executes. Is there an order of execution that will allow different threads to get the same number back from getNum()?

Quote:

---

Originally Posted by Norm

Why not? Please explain why you think that.
The OS determines when each of the threads executes. Is there an order of execution that will allow different threads to get the same number back from getNum()?

---

Yeah I understand that the OS scheduler decides which thread runs when but the reason why I want to SYNCHRONIZE is to have a shared resource that when the thread executes it will add 1 to the NUM variable and then to return the NUM value so this will show that THREAD 1 = 1 for example but I dont understand why the threads are not locking the shared resource to increment and then return then move onto the next thread.

The threads are NOT locking the resource between the calls to the two methods.

Try debugging the code by adding printlns to the two methods that show the value of num and the name of the thread that is executing. The print out should show you the order of the calls to the methods.

Think I have sorted it. Took on board what you said about the resource not locking between the methods so it could be calling getNum() before actually incrementing etc.

I have changed my code so that the variable is incremented and value is returned within the same synchronized method and seems to do the trick as I have ran it a few times and not got any duplicate numbers.

Is this now correct so that the shared resource is locked so that one thread must finish the addAndReturn() method before another thread can process this?

Shared Class:

Code :

---

package threads;
 
public class Shared
{
    private static int num = 0;
 
 
    public synchronized static int addAndReturn()
    {
        num++;
        return num;
    }
 
}

---

Threads Class:

Code :

---

package threads;
 
public class Threads extends Thread
{
    private String Tname;
 
    public Threads(String name)
    {
        Tname = name;
    }
 
    public void run()
    {
         System.out.println("Thread: " + Tname + " is number: " + Shared.addAndReturn());
    }
 
}

---

Main Class:

Code :

---

package threads;
 
public class StartThreadsMain
{
 
    public static void main(String[] args)
    {
        Threads t1 = new Threads("T1");
        Threads t2 = new Threads("T2");
        Threads t3 = new Threads("T3");
        Threads t4 = new Threads("T4");
 
        t1.start();
        t2.start();
        t3.start();
        t4.start();
 
    }
 
}

---

Output:

Code :

---

run:
Thread: T1 is number: 1
Thread: T2 is number: 2
Thread: T3 is number: 3
Thread: T4 is number: 4
BUILD SUCCESSFUL (total time: 0 seconds)

---

Output where threads run at different time but still a correct output:

Code :

---

run:
Thread: T3 is number: 2
Thread: T1 is number: 1
Thread: T4 is number: 3
Thread: T2 is number: 4
BUILD SUCCESSFUL (total time: 0 seconds)

---

Yes the new code would keep the incrementing and getting the value in synch.

Quote:

---

it could be calling getNum() before actually incrementing etc.

---

No, in any one thread, the call to getNum() is after the call to addToNum(). That would not be changed.

so really the synchronized method still isnt doing anything as I have removed this and I am still getting the correct output.

Quote:

---

synchronized method still isnt doing anything

---

Yes, probably not.
Your computer is too fast. Each thread completes before the next one starts.
Put some work into the methods that would slow the thread down and allow other threads to catch up.

Quote:

---

Originally Posted by Norm

Put some work into the methods that would slow the thread down and allow other threads to catch up.

---

isnt that just tricking it though, in reality it should work how its programmed so I still have something wrong

Do you understand how fast your computer is?

Quote:

---

in reality it should work how its programmed

---

Please explain what you mean here?

I think it is working the way it is programmed.

Ok

I will start from the beginning, what I am trying to achieve is to have multiple threads that all have a shared resource (the NUM variable in the shared class).

I then need the threads to manipulate that in some way (currently incrementing this by 1).

But the threads are not allowed to touch that value (either get or set) until the previous thread has finished getting or setting (so that value is correct before the next thread starts processing).

Is it just the fact that I dont have enough threads starting to prove that the synchronized keywork is working in my changed scenario?

Quote:

---

prove that the synchronized keywork is working

---

The CPU is too fast. You need a way to slow down the threads so other threads can catch up.
With your very short methods, how can one thread catch the other? The first thread will be done before the next thread starts.

Quote:

---

Originally Posted by Norm

The CPU is too fast. You need a way to slow down the threads so other threads can catch up.
With your very short methods, how can one thread catch the other? The first thread will be done before the next thread starts.

---

Your right :)

That took a bit to sink in ha ha :)

I have now added a Thread.sleep and without the synchronized keyword all the threads return 4 as they can run that method at the same time but by adding the synchronized keyword then they return as they should and no duplication of number :)

Thanks a lot and sorry for being a bit dopey.

Shared class:

Code :

---

package threads;
 
public class Shared
{
    private static int num = 0;
 
 
    public synchronized static int addAndReturn()
    {
        num++;
        try
        {
            Thread.sleep(2000);
        } 
        catch (InterruptedException ex)
        {
            System.out.println(ex.getMessage());
        }
        return num;
 
    }
 
 
}

---

Threads class:

Code :

---

package threads;
 
public class Threads extends Thread
{
    private String Tname;
 
    public Threads(String name)
    {
        Tname = name;
    }
 
    public void run()
    {
         System.out.println("Thread: " + Tname + " is number: " + Shared.addAndReturn());
    }
 
}

---

Main class:

Code :

---

package threads;
 
public class StartThreadsMain
{
 
    public static void main(String[] args)
    {
 
        Threads t1 = new Threads("T1");
        Threads t2 = new Threads("T2");
        Threads t3 = new Threads("T3");
        Threads t4 = new Threads("T4");
 
 
        t1.start();
        t2.start();
        t3.start();
        t4.start();
 
 
    }
 
}

---

Output without Synchronized keyword:

Code :

---

run:
Thread: T1 is number: 4
Thread: T2 is number: 4
Thread: T3 is number: 4
Thread: T4 is number: 4
BUILD SUCCESSFUL (total time: 2 seconds)

---

Output with Synchronized keyword:

Code :

---

run:
Thread: T1 is number: 1
Thread: T4 is number: 2
Thread: T3 is number: 3
Thread: T2 is number: 4
BUILD SUCCESSFUL (total time: 8 seconds)

---

Glad you got it working.