CODEWITHSUNDEEP
javamultithreadingthread-safety
Quincy
Quincy

Reputation: 1939

Java object implements Runnable, how to remove the object from a collection

I have a java object that implements Runnable. here is the code.

public class Obj implements Runnable {

    boolean shouldRun;

    private int stuff

    public Obj() {
        this.setshouldRun(true);
        stuff = 0;
    }

    public synchronized void setshouldRun(boolean shouldRun) {
        this.shouldRun = shouldRun;
    }

    public synchronized boolean getshouldRun() {
        return this.shouldRun;
    }

    @Override
    public void run() {
        while (shouldRun) {
            //do stuff
        }
}

}

here is how i use the Obj.

Obj o = new Obj();
//q is a collection of Obj
q.add(o);
new Thread(o).start();

when want to remove o, may I safely do this:

o.setshouldRun(false); //thread will finish
q.remove(o);

Upvotes: 1

Views: 2937

Answers (4)

Quincy
Quincy

Reputation: 1939

I think using java TimerTask would be a better solution. TimerTask could be cancelled by other threads. so the TimeTask thread will exist pretty much immediately, so no extra work would be done.

Upvotes: 0

Matt Wonlaw
Matt Wonlaw

Reputation: 12443

The best bet would be to define shouldRun as volatile as synchronizing is slower. Assignments are not guaranteed to propagate to other threads unless dictated by the use of volatile and synchronize.

If you have access to o's thread, you can get rid of your boolean "shouldRun" by just using the thread's native "interrupt" method:

Obj o = new Obj();
q.add(o);
Thread t = new Thread(o);
t.start();

t.interrupt();
q.remove(o);

Your runnable would then have to read:

while (!Thread.interrupted()) {
}

Instead of:

while (shouldRun) {
}

Upvotes: 1

Zak
Zak

Reputation: 25205

As long as nothing comes after q.remove(o) that would depend on o not being in a running state, you should be ok. But remember, the way you have your code, o can continue running long long after it is removed from q, because you haven't stopped to check that it has finished running before removing it from q. You've only notified it that it should stop running once it gets back around to checking that condition.

Upvotes: 1

Mark Byers
Mark Byers

Reputation: 838376

You need to synchronize all access to shouldRun, not just write access. You can do this by adding a synchronized getter isShouldRun rather than accessing shouldRun directly on this line:

while (isShouldRun())

Alternatively you can make shouldRun volatile.

Failing to do this can lead to changes to the value of shouldRun by one thread not being noticed in the other thread.

Upvotes: 3

Related Questions