synchronization : Threads execute two critical sections in same order

Question

I have the following kind of code:

synchronized block1 {
    //only one thread in the block
}

{lot of code where synchronization not necessary}

synchronized block2 {
    //only one thread in the block. 
    //All the threads that executed block1 before this thread should have already executed this block.
}

Each thread first executes block1, non synchronized block, and block2 in that same order.

If thread T1 executes block1 before thread T2, then T1 should execute block2 before T2. There are more than two threads.

Is there a way to achieve this in java?

Sceptic

I take it that you cannot call block2() inside block1? – Rebba

@NicholasRobinson No calls. All the code is executed in the given order. – Sceptic

Is this in one method? Using synchronized(this) {...} or are they different methods? – Rebba

@NicholasRobinson Everything is in one method. The {...} is only to show the seperation between the synchronized and non synchronised blocks – Sceptic

@NagarjunaSiddam Is your number of threads fixed to 2? – Douro

@Douro The number of threads can be more than 2. – Sceptic

Answer 1

As I understand Critical Section #2 MUST be executed in the same order as Critical Section #1

If thread T1 executes block1 before thread T2, then T1 should execute block2 before T2. There are more than two threads.

Then a Queue might be used to ensure the order of execution.

private Object lock = new Object();
private Queue<Thread> threadQueue = new ArrayDeque<>();

// https://mcmap.net/q/1374837/-synchronization-threads-execute-two-critical-sections-in-same-order
public void executeCriticalSectionsInOrder() throws InterruptedException {
    // Critical Section #1
    synchronized (lock){
        // synchronized code #1

        // Add self to queue
        threadQueue.add(Thread.currentThread());
    }

    // {lot of code where synchronization not necessary}

    // Critical Section #2
    synchronized (lock) {
        //All the threads that executed block1 before this thread should have already executed this block.
        // Wait turn
        Thread t = threadQueue.element(); // Do not remove until it is self
        while (t != Thread.currentThread()) {
            lock.wait();
            // After sleep try again
            t = threadQueue.element();
        }
        // Verified own turn. Update status
        threadQueue.remove();

        // synchronized code #2

        lock.notifyAll(); // Awake any waiting thread after exiting section.
    }

However If one thread dies/exits without removing itself from the queue, then following threads will be blocked indefinetely. Maybe add a finally block to do the housekeeping?

Note: In Nicholas Robinson's answer a position order was suggested instead of a queue, which seems slightly more efficient.

Epigenous

Answer 2

This basically creates a queue that threads will wait in until their number comes up. [UPDATED]

private AtomicInteger place = new AtomicInteger(0);
private AtomicInteger currentPlaceInQueue = new AtomicInteger(0);
private ReentrantLock lock = new ReentrantLock();
private Condition notNext = lock.newCondition();

public void method() {

   ThreadLocal position = new ThreadLocal();

   synchronized(this) {
      //Your code
      position.set(place.getAndIncrement());
   }

   // More code

   lock.lock();
   while ((int) currentPlaceInQueue.get() != position.get()) {
      notNext.await();
   }
    // More code
   lock.unlock();
   currentPlaceInQueue.getAndIncrement();
   notNext.notifyAll();
 }

Rebba

Just check, the ThreadLocal definition might have to be an object variable. – Rebba

I don't see why ThreadLocal is required - it is scoped to a method, not the class. – Daugavpils

@AndyBrown I suppose you can just you an int variable in the method. Should give the same result. But I did comment that ThreadLocal might need to be an object variable. – Rebba

+1 for good attempt with interesting concepts (Use order number instead of queue). However 1) Where you check place.get() you might mean position.get(). 2) IMHO there is no need for a secondary overlapping lock. Also you might mean notNext.signalAll(). 3) Also I would had gone with local position and not atomic place and currentPlaceInQueue (updating and accessing from synchronized) – Epigenous

Answer 3

The synchronized blocks in your example are a red herring. Your problem is, you have N threads, and you have two blocks of code, and you want to make sure that none of the threads enters the second block until all of them have finished the first block.

You use a CyclicBarrier for that. http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CyclicBarrier.html

Sevenup

Answer 4

You should be able to use a Lock which you take before calling block1 and release after calling block2.

static Lock lock = new ReentrantLock();
Random random = new Random();

public void block1() throws InterruptedException {
    System.out.println("Enter block 1");
    Thread.sleep(random.nextInt(500));
    System.out.println("Leave block 1");
}

public void block2() throws InterruptedException {
    System.out.println("Enter block 2");
    Thread.sleep(random.nextInt(500));
    System.out.println("Leave block 2");
}

private class BlockTester implements Runnable {

    long start = System.currentTimeMillis();

    @Override
    public void run() {
        while (System.currentTimeMillis() < start + 10000) {
            lock.lock();
            try {
                System.out.println("Thread: " + Thread.currentThread().getName());
                block1();
                block2();
            } catch (InterruptedException ex) {
                System.out.println("Interrupted");
            } finally {
                lock.unlock();
            }
        }
    }
}

public void test() throws InterruptedException {
    Thread[] blockTesters = {
        new Thread(new BlockTester()),
        new Thread(new BlockTester()),
        new Thread(new BlockTester()),
        new Thread(new BlockTester()),
        new Thread(new BlockTester())
    };
    for (Thread t : blockTesters) {
        t.start();
    }
    for (Thread t : blockTesters) {
        t.join();
    }

}

Anemochore

You are calling methods block1() and block2() one after other, cant we keep them in single method? – Douro

Don't see how this satisfies the OP - he WANTS multiple threads to go through block1 and the other unsync'ed code - what he doesn't what is one thread to overtake another and get to block 2 first. Specifically he wants to suspend a thread at block 2 if another thread which passed block 1 before it hasn't gone yet. – Homesteader

How are you handling this: ` {lot of code where synchronization not necessary}` – Douro

you are locking around the class of those supposed to be synchronized blocks, but you are also locking on the coder that dosent need to be synchronized. – Rattlebrain

@All - Doesn't If thread T1 executes block1 before thread T2, then T1 should execute block2 before T2 mean that no other thread can access block2 until T1 has finished block2? – Anemochore

@Anemochore this is definitely a solution, but OP requires that the code between block1() and block2() (missing in your answer) may be executed in parallel. (no -1 from me) – Athletics