CODEWITHSUNDEEP
javamultithreading
All_Safe
All_Safe

Reputation: 1399

Safe stop thread

I have some class:

@Component
public MyClass {
   private volatile boolean stopped = false; 

   public void verification() throws Exception {

        Thread kpiAllThread = getKPIAllThread();

        try {
            for (int i = 0; i < poolSize; i++) {
                execDispatcher.put(processExecutor.submit(getCheckValuesInKPIConsumerTask(workingQueue)));
            }
            kpiAllThread.start();
        } finally {
            waitFinished();
        }
    }

    public void setStop(bolean stopped) {
         this.stopped = stopped;
    }

    private Thread getKPIAllThread() {
        return new Thread(() -> {
            try {
                LOG.debug("KPIAllThread started!");
                dao.getKpiAll(workingQueue);
                for (int i = 0; i < poolSize; i++) {
                    workingQueue.put(() -> true);
                }
            } catch (Exception ex) {
                LOG.error("KPIAllThread exception: ", ex);
            } finally {
                LOG.error("KPIAllThread finished!");
            }
        });
    }
}

This class starts the producer thread getKPIAllThread. He get data from db and put in BlockingQueue.

Method getKpiAll like this:

public void getKpiAll(final BlockingQueue<KeyPropertyIndex> kpiData) throws Exception {
        LOG.debug("Starting getKpiAll");
        try (final Connection con = dataSource.getConnection();
             final Statement stmt = con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) {
            stmt.setFetchSize(Integer.MIN_VALUE);

            try (final ResultSet rs = stmt.executeQuery(sqlGetAllkpi)) {
                while (rs.next()) {
                    kpiData.put(new KeyPropertyIndexData(rs.getLong(1), rs.getString(2)));
                }
            }
            LOG.debug("Finished get getKpiAll");
        } catch (Exception ex) {
            throw ex;
        }
    }

There is also a variable stopped that can be set from outside to true. How can I safely stop my thread while doing so? So that all connections to the database are closed and the thread is successfully completed?

Upvotes: 2

Views: 108

Answers (1)

jurez
jurez

Reputation: 4657

The cleanest and safest rule for stopping a thread is that the code running in thread should periodically check a condition (say, boolean shouldExit()). When the code detects that this condition is true, it should stop doing what is doing and terminate.

The code running in thread should check this condition fairly often so that it can react reasonably fast. As a rule of thumb, the thread should exit less than one second after you set this condition. The check would typically look something like if (shouldExit()) break somewhere in your for-loop that iterates over pool size. However, dao.getKpiAll(workingQueue) looks potentially long, so you might place more checks inside getKpiAll.

When you have this checking in place, you must ensure that your code will exit cleanly every time the condition becomes true. For example, you can use finally blocks to close any connections etc. If this happens during getKpiAll, there is no sense to even continue with for loop to process items and so on.

Sometimes, this can get more tricky - i.e. when the thread is waiting on a network operation, you might need to close the network socket or something like that to interrupt it. In any case, avoid using Thread.stop() or Thread.interrupt() - see documentation why they are problematic.

If you do things like this, you can set the condition from outside the thread at any time to request the thread to terminate. You can make something like void requestExit() and set a boolean variable there. After calling requestExit(), you call Thread.join() with a suitable timeout to wait for the thread to do its business, check the condition and exit. Again, as a rule of thumb, make the timeout 3-10 times as long as the longest reaction time of your thread.

It looks that you already have setStopped(boolean stopped) for that purpose, but you're not checking it. First, I would remove parameter stopped because it doesn't make sense to pass false to it. Second, you need to add checks as described above. You might want to make this variable visible to dao - just remember that it's much better to expose it as a synchronized boolean method than as a boolean field.

Upvotes: 2

Related Questions