Instead of using synchronized keyword i am using lock concept but i am not getting the output as i got in synchronized key word?

Question

    import java.util.concurrent.locks.ReentrantLock;

class Displayx
{
    public void wish(String name)
    {
        ReentrantLock lock = new ReentrantLock();
        //using locks
        lock.lock();
            for (int i = 0; i < 10; i++) 
            {
                System.out.print("Good Morning : ");
                try {
                    Thread.sleep(2000);
                } catch (InterruptedException e) {
                    System.out.println("I got intruppted");
                }
                System.out.println(name);
            } 
        lock.unlock();
    }
}
class MyThreadex2 extends Thread
{
    Displayx d;
    String name;
    public MyThreadex2(Displayx d, String name) 
    {
        this.d = d;
        this.name = name;
    }
    @Override
    public void run() 
    {
        d.wish(name);
    }
}
public class ReentrantLockDemo1 {

    public static void main(String[] args) 
    {
        Displayx d = new Displayx();
        MyThreadex2 mt1 = new MyThreadex2(d, "SHOAIB");
        MyThreadex2 mt2 = new MyThreadex2(d, "RAHUL");

        mt1.start();
        mt2.start();

    }

}

output i am getting is

Good Morning : Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : RAHUL
Good Morning : SHOAIB
Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : RAHUL
Good Morning : SHOAIB
Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : RAHUL
Good Morning : SHOAIB
Good Morning : SHOAIB
Good Morning : RAHUL
Good Morning : RAHUL
SHOAIB

Use the **same** `Lock`? Just like you would need to `synchronize` on the same `Object`. In fact in this case there is no benefit to using `Lock` - it comes into its own when locks needs to interleaved... — Boris the Spider, Nov 19 '16 at 14:16
Perhaps more importantly, **don't `extends Thread`**. (unless you know what you are doing and have good cause) — Boris the Spider, Nov 19 '16 at 14:17
@BoristheSpider Why not `extends Thread`? Please link to article explaining why. — Andreas, Nov 19 '16 at 14:54
[There you go](http://stackoverflow.com/q/541487/2071828) @Andreas. — Boris the Spider, Nov 19 '16 at 15:21
I see recommendations, about flexibility, reuse, coding to interface, and the like, but your *"unless you know what you are doing and have good cause"* makes it sound *dangerous* to extend Thread, and I don't see that. Maybe I just misinterpreted what you said? — Andreas, Nov 19 '16 at 15:29
@Andreas it can be rather dangerous - for example using `wait` and `notify` on a `Thread` can have rather unexpected consequences as `Thread` notifies itself internally on death. It's generally best avoided for all sorts of reasons. — Boris the Spider, Nov 19 '16 at 15:35

score 0 · Accepted Answer · answered Nov 19 '16 at 14:22

Move ReentrantLock lock = new ReentrantLock(); outside of your method.

ReentrantLock lock = new ReentrantLock();
         public void  wish(String name)
         {
             //using locks
             lock.lock();
             try {
                 for (int i = 0; i < 10; i++) {
                     System.out.print("Good Morning : ");
                     try {
                         Thread.sleep(2000);
                     } catch (InterruptedException e) {
                         System.out.println("I got intruppted");
                     }
                     System.out.println(name);
                 } 
             } finally {
                 lock.unlock();
             }

         }

Vasu · Answer 2 · 2016-11-19T14:28:42.830

In Java, variables created inside the method are accessible (scoped) within the same thread.

As your ReentrantLock object is created inside the wish() method, it is local to each thread. i.e., you are actually creating individual lock objects for each thread, because of which multiple threads are entering into your critical section code (inside the try block).

class Displayx
{
    ReentrantLock lock = new ReentrantLock();//Use the same lock object

    public void wish(String name)
    {
        //using locks
        lock.lock();
        try {
            for (int i = 0; i < 10; i++) {
                System.out.print("Good Morning : ");
                try {
                    Thread.sleep(2000);
                } catch (InterruptedException e) {
                    System.out.println("I got intruppted");
                }
                System.out.println(name);
            } 
        } finally {
            lock.unlock();
        }
    }
}

Also, as Boris mentioned in the comment, make sure that you are "coding to interfaces" by using Runnable or Callable interfaces instead of directly extending Thread class.

Instead of using synchronized keyword i am using lock concept but i am not getting the output as i got in synchronized key word?

2 Answers2