Weird things happen when I try to kill a Thread in java?

user_15458534 0
Viewed 93

I have this "Serial" class that extends Thread that works with a serial port. In the run() method I first open the port, set all the Serial params and add a SerialPortEventListener. Then i just put a while(go) where go is a private (so only the class can see it) boolean instance set as true. I have a kill method that not only it closes the port and remove the SerialEventListener, but also set "go" as false, hence the Thread should stop. Now, here is where things start getting weird: if I put a System.out.println(something) in the while(go), when I call the kill() method the Thred stops, but if I doesn't it remains alive. Do you have any ideas about why it works like that and how to solve it?

while(go) {
            System.out.println("in");
        }

public void kill(){
        go = false;
        try {
            this.serialPort.removeEventListener();
            this.serialPort.closePort();
        } catch (SerialPortException e) {
            e.printStackTrace();
        }
    }
multithreading java
2 Answers

Unless go is declared volatile, there is no guarantee the while loop (or anything else) will read the current value.

declare go as

volatile boolean go;

Then it should work (unless there are other things going on that haven't been posted). And doing this does not alleviate the requirement to do proper synchronization.

user_15525340

I am not going to arguing about the way you implemented those thread. However, lookig at the specific scenario, you have a the Serial class, which stays alive and does its job as long as the variable go is true. The same variable can also be changed by another thread (i.e. the main one) once the method kill is invoked.
As suggested by @WJS, the best approach to make the things work properly, is to declare the variable go as "volatile". You should have something like that:

public class Serial extends Thread {
   ...
   private volatile boolean go;
   ...

   public void kill(){
      ...
   }
   ...
}

Basically, in this way, the JVM makes sure that different threads read a consistent value of the variable, and that actually it's what happend in a multithread scenario like yours.

Another approach is to define the variable go as java.util.concurrent.atomic.AtomicBoolean, instead of having it as simple primitive boolean. That also works very well in this situation, because in this way every change of the variable's value is guaranteed by the JVM as being an atomic operation, which means made it "at ones", and you will always avoid any possible race condition.
In this case, you code should be something like that:

import java.util.concurrent.atomic.AtomicBoolean;
...
public class Serial extends Thread {
    private AtomicBoolean go = new AtomicBoolean(true);
    ...

    public void loop() {
        ...
        while(go.get()) {
        
        }
    }
 
    public kill() {
       atomicBoolean.set(false);
       ...
    }
}
user_55466730
Related