Should I create an abstract runnable? I've 4 sub-classes which perform similar work but only 3 of them have identical constructors

I've added the code for an abstract thread class and 2 sub-classes. The structure and job of the threads (sub-classes) is identical except one difference: ONE of the sub-classes, RefreshData does not need the client parameter. I've overloaded the constructor in the sub-class but I'm not sure if the abstraction makes sense anymore. Moreover, can I do something to ensure that RefreshData doesn't use the inherited client parameter accidentally and ends up getting an NPE.

public abstract class BackgroundTask implements Runnable {

      protected final Manager manager;
      protected final Client client;
      private long msSyncInterval;
      private boolean shutdown; // has a setter 

    public BackgroundTask(final Manager manager, final long msSyncInterval) {
        this.manager = manager;
        this.client = null;
        this.msSyncInterval = msSyncInterval;
        this.shutdown = shutdown;
    }

    public BackgroundTask(final Manager manager, final Client client, final long msSyncInterval) {
        this.manager = manager;
        this.client = client;
        this.msSyncInterval = msSyncInterval;
        this.shutdown = shutdown;
    }

    @Override
    public void run() {
        while (!shutdown) {
            executeTask();

            // sleep for msSyncInterval
        }
    }

    abstract void executeTask();
}

public class ExchangeData extends BackgroundTask {

    public ExchangeData(final Manager manager, final Client client, final long msSyncInterval) {
        super(manager, client, msSyncInterval);
    }

    @Override
    void executeTask() {
        // Some work here
    }
}

public class RefreshData extends BackgroundTask {

    public RefreshThrottleLimits(final Manager manager, final long msSyncInterval) {
        super(throttleManager, msSyncInterval);
    }

    @Override
    void executeTask() {
        // Doesn't use the client
    }
}

You have field shutdown that you mutating in a multithreaded code, this field can be cached and might ignore you changing it. You can declare it as volatile to avoid this behavior.

private volatile boolean shutdown;

What makes your abstraction leaky is the fact that BackgroundTask is not abstract enough, that is, Manager and client shouldn't be there.

public abstract class BackgroundTask implements Runnable {

   abstract void executeTask();
   private boolean shutdown;

   @Override
   public void run() {
    while (!shutdown) {
        executeTask();

        // sleep for msSyncInterval
    }
  }
 }

And now we can have an abstract class that does Task involving clients and managers

abstract class AbstractManagerClientTask extends BackgroundTask { // not sure about the name
   protected final Manager manager;
   protected final Client client;
   .....
 }

ExchangeData is more specific than a BackgroundTask, it is a AbstractManagerClientTask

class ExchangeData extends AbstractManagerClientTask{
   ...
  }

It's much easier now, RefreshData can extend BackgroundTask with its own Manager field

public class RefreshData extends BackgroundTask {
  private final Manager manager;
  ....
 }

StackExchange Code Review Q#69179, answer score: 2