Synchronized block still locked after thread restart

83 views Asked by At

I try to restart thread but synchronized block in thread keep locked after restarted. I shouldn't change socket properties because some processes take too long but when network connection lost it hangs forever. I try to use InterruptedException but it doesn't work. Is there any way to release this lock?

public static void main(String[] args) {
    try {
        synchronizedBlock t1 = new synchronizedBlock();
        t1.start();
        Thread.sleep(500);
        t1.cancel();
        t1 = new synchronizedBlock();
        t1.start();
    } catch (Exception e) {
        e.printStackTrace();
    }
    while (true) {

    }
}

public class synchronizedBlock extends Thread {

   boolean isRunning = true;
   boolean isRunning2 = true;
   public static Object[] locks = new Object[5];

   public synchronizedBlock() {
       for (Integer i = 0; i < 5; i++) {
           synchronizedBlock.locks[i] = i;
       }
   }

   public void cancel() {
       isRunning = false;
       interrupt();
   }

   public void socketProces() {
       while (isRunning2) {
       }
   }

   public void proces(int index) {
       try {
           synchronized (locks[index]) {
               System.out.println("Synchronized Block Begin");
               socketProces();
           }
       } catch (Exception e) {
           e.printStackTrace();
       }
   }

   @Override
   public void run() {
       try {
           System.out.println("Run begin");
           while (isRunning) {
               proces(1);
           }
           Thread.sleep(1);
       } catch (InterruptedException e) {
           //Do Something
       } catch (Exception e) {
           e.printStackTrace();
       }
   }
}

Result:

Run begin
Synchronized Block Begin
Run begin
2

There are 2 answers

0
polo-language On

When you start the synchronizedBlock thread you'll get a stack trace like this I think: run -> proces -> socketProcess. Then because isRunning2 = true, the thread will enter an infinite loop in socketProcess and never terminate.

Keep in mind that in Java there is no such thing as 'restarting' a thread. Once started, a thread can never be restarted. Indeed, you are creating two sycnchronizedBlock objects, not restarting a single object.

As a side note, it is generally problematic to overwrite static state in a class constructor, as you're doing with the locks variable, without synchronization.

0
Progman On

The issue here is the Integer cache which is used in the for loop to initialize the synchronizedBlock.locks array:

for (Integer i = 0; i < 5; i++) {
    synchronizedBlock.locks[i] = i;
}

When this code is run again, due to the constructor of the second synchronizedBlock, the synchronizedBlock.locks array contains the same Integer instances which where created when this for loop was executed for the first time. This means that the synchronized (locks[index]) lock will be on the same Integer object. As you have already one thread holding the lock for the Integer(1) object, the second thread waits outside the lock waiting for it to be released.

This is also problematic in combination with the fact that the first thread is not terminating. Your method

public void socketProces() {
    while (isRunning2) {
    }
}

is an endless loop as you don't change the value of isRunning2, ever. Also, the interrupt() method itself does not stop any thread. Instead, it sets just an internal flag in the Thread class, which can be checked with isInterrupted() and interrupted(). You have to check this flag and react on it like "Oh, someone wants me to stop, so I stop now".

To solve your problem you should at least quit your thread when the "isInterrupted" flag of the Thread instance is set. You can do it like this:

public void socketProces() {
    while (isRunning2) {
        if (Thread.interrupted()) {
            return;
        }
    }
}

Instead of returning from socketProces() normally you could throw an InterruptedException like other methods do.

Also, depending on how you want to initialize/use the instances you want to lock on with synchronized(...), you might want to consider on how you create/fill the synchronizedBlock.locks array and which objects you want to use (the Integer cache might be problematic here). It depends on you if the creation of a new synchronizedBlock instance will/should/shouldn't create new objects to lock on in the synchronizedBlock.locks array.