CODEWITHSUNDEEP
javamultithreadingsemaphore
ExR
ExR

Reputation: 25

Can't synchronise threads in Java (using semaphores)

Good day!

I have met problem with synchronizing threads. I am writing program which is like dinner philosophers. I have few processes (3 for example) and resources (4 for example). Each process can work only with 2 free resources. That means that 1st process can work only with 1st and 2nd resources and etc.

I decided to use semaphores for my aim. Problem is that is still doesn't have synchronization. For example if 1st and 3rd processes are working with resources then 2nd process must wait until his resources will not be release. In my program sometimes it happens... Sometimes it doesn't.

What is the problem? How can i solve that?

Code is here:

public class Sem
{
    public Sem()
    {
        available = new ConcurrentHashMap< Integer, Semaphore >();//Resources.

    for ( int i = 1; i <= 4; i++)
    {
        available.put( i, new Semaphore(1, true) );//Each resource contains semaphore.
    }
}

public void start( final int id )
{
    thisThread = new Thread()
    {
         public void run()
         {
            try
            {
                work( id );                                                 //Try to take resourses.
                Thread.currentThread().sleep(1000);
                release( id );                                              //Release resources.
            } catch (InterruptedException ex) {
                Logger.getLogger(Sem.class.getName()).log(Level.SEVERE, null, ex);
            }

         }
    };
    thisThread.start();
}

public synchronized void work( int id ) throws InterruptedException
{
    available.get(id).acquire();                                            //Try to take resourse[id] and resourse[id+1]
    available.get(id+1).acquire();                                          //Thread is blocking till it will be possible.

    System.out.printf("Acquired [%s], id = %d\n",Thread.currentThread().getName(), id);

}

public void release( int id )
{
    available.get(id).release();                                            //Release resources which hava been captured by thread.
    available.get(id+1).release();                                          //After this other thread can take these resourses.

    System.out.printf("Released [%s], id = %d\n",Thread.currentThread().getName(), id);
}


private ConcurrentHashMap< Integer, Semaphore > available;                  //Integer - id of thread[1..4]; Semaphore - is gate with param (1)
                                                                            //Available - map of resources which can be busy by processes.
Thread thisThread;
}

I start this program like this:

Sem sem = new Sem();

        sem.start(1);
        sem.start(2);
        sem.start(3);

I have few output messages but my favorite:

Acquired [Thread-1], id = 1
Acquired [Thread-3], id = 3
Released [Thread-1], id = 1
Acquired [Thread-2], id = 2
Released [Thread-3], id = 3
Released [Thread-2], id = 2

Process 2 started worked while he can't do it!!

Upvotes: 0

Views: 1186

Answers (3)

Mak
Mak

Reputation: 614

You are finishing with the resource first then the thread so release the resource semaphore first. Replace the release method with followings:

public void release(int id) {
    resources.get(id + 1).release();
    resources.get(id).release();  
    //resources.get(id + 1).release();
    System.out.printf("Released [%s], id = %d\n", Thread.currentThread().getName(), id);
}

Upvotes: 0

Alex Gitelman
Alex Gitelman

Reputation: 24732

Based on your code and output, it is possible that Thread 3 released semaphore 3 so Thread 2 that was waiting for it - acquired it and printed the message before Thread 3 completed.

I noticed that you didn't synchronized release method.

I highly recommend putting synchronized block around both acquire and release.

synchronized(this) {
    available.get(id).acquire();                                            
    available.get(id+1).acquire();                                         
}

synchronized(this) {
   available.get(id).release();                                            
   available.get(id+1).release(); 
}

Upvotes: 1

duffymo
duffymo

Reputation: 308998

I think you should release the locks in reverse order in which they were obtained.

Upvotes: 3

Related Questions