Stop flag in a multithreaded handler

Question

Java

Good afternoon.
Despite the fact that I have been writing in Java for a long time, I am not a specialist in the field of multi-threaded programming. Therefore, I want to consult with more experienced colleagues.
Task:
There is a certain class that processes requests in multi-threaded mode (service business method). We need to make a stop method (let's call it destroy). It should work as follows:
You need to make sure that after calling the destroy method, new requests are not processed, the processing of old ones is successfully completed, and after the completion of the last processing, some actions are performed.
I did like this:

public class Worker{
    AtomicBoolean flag;
    AtomicInteger counter;

    public void destroy() {
  flag.set(true);
  if (counter.get() == 0) {
      destroyImpl();
  }
    }

    public void service(String arg) {
  if (!flag.get()) {
      counter.incrementAndGet();
      if (!flag.get()) {
    serviceImpl(arg);
      }
      counter.decrementAndGet();
  } else {
      if (counter.get() == 0) {
    destroyImpl();
      }
  }
    }

}

I suspect that not only clumsily, but also buggy.
How right?

Answer 1

Completely wrong use of atomic operations. The destroyImpl() method, for example, can be called multiple times. Why don't you use classic locks and get rid of atomic entities? It's easier and much more readable. In addition, you need to work with several fields at once - and this is not trivial if you use only atomic operations.
You should get something like this:

public class Worker {

    private boolean destroy;
    private boolean destroyed;
    private counter;

    public void destroy() {
        synchronized (this) {
            if (this.destroyed) {
                return;
            }
            this.destroy = true;
            if (this.counter != 0) {
                return;
            }
            this.destroyed = true;
        }
        destroyImpl();
    }

    public void service(String arg) {
        synchronized (this) {
            if (this.destroy) {
                return;
            }
            ++this.counter;
        }
        serviceImpl(arg);
        synchronized (this) {
            if (--this.counter != 0) {
                return;
            }
            if (!this.destroy) {
                return;
            }
            this.destroyed = true;
        }
        destroyImpl()
    }

}

Answer 2

Use ThreadPoolExecutor It has a method shutdown()that, after the call, will finish the “tasks” that you gave it to execute, but will no longer accept new ones.
Tasks can be, for example, or Callable<T>if you want to return a value orRunnable

Answer 3

I read the question several times, but did not understand - do you have 1 Worker instance for many threads? If you synchronize on it, then you will get the usual sequential execution, why then multithreading at all. Clarify please.