I have a singleton class which extends from an abstract java class. Two singleton classes extend from ItemImageThreadManager, the reason for this is to use shared scheduling functionality. A thread is created and passed into the runThread method of ItemImageThreadManager.Is this good design?Abstract Parent Classpublic abstract class ItemImageThreadManager {    private Timer timer = new Timer();    public void runThread(final Thread runThread){        try {             scheduleTimer(runThread);        }        catch(IllegalStateException ise){            /**             * When the timer is cancelled (when screen closed) and the images are              * removed from memory(in-app resync) and this screen is opened again,              * an IllegalStateException will be thrown since the images will need              * to be re-downloaded             *              * Solution - create a new timer.             */            timer = new Timer();            scheduleTimer(runThread);        }    }    /**     * Starts a new thread. This thread is run in the order is has been added.     * Since timer runs in it's own thread, this gaurantees that just one thread      * will be started at one time....     * @param thread     */    private void scheduleTimer(final Thread thread){        timer.schedule(new TimerTask() {            public void run() {                thread.start();                try {                     thread.join();                } catch (InterruptedException e) {                    e.printStackTrace();                }            }        }, 0);    }    public void cancelTimer(){        timer.cancel();    }}First Extended class public class ContestantImageThreadManager extends ItemImageThreadManager{    private static ContestantImageThreadManager ref;    private ContestantImageThreadManager(){        super();    }    public static ContestantImageThreadManager getSingletonObject()    {      if (ref == null){          ref = new ContestantImageThreadManager();      }      return ref;    }}Second Extended classpublic class JudgeItemImageThreadManager extends ItemImageThreadManager{    private static JudgeItemImageThreadManager ref;    private JudgeItemImageThreadManager(){    }    public static JudgeItemImageThreadManager getSingletonObject()    {      if (ref == null){          ref = new JudgeItemImageThreadManager();      }      return ref;    }}
Singleton class extending a parent class to utilise shared functionality
java;design patterns;multithreading;thread safety;singleton
As others mentioned the getSingletonObject should be synchronized. There is a chapter in Effective Java, Second Edition about singletons (Item 3: Enforce the singleton property with a private constructor or an enum type). It's worth to read. Anyway, try to avoid them, they makes testing really hard.You use the Timer as a single-threaded executor. In that case consider using Executors.newSingleThreadExecutor. Furthermore, are you sure that you need to pass Thread at all? Wouldn't Runnable be enough? Executors can run Runnables without any wrapper class and Thread.join call.Instead of catching IllegalStateException in the runThread method you should check whether the timer is canceled. You can store this state in a boolean flag. Don't forget to synchronize the variable access. (See Effective Java, Item 57: Use exceptions only for exceptional conditions)ExecutorService.submit returns Future which has a cancel method. Maybe it's better for you requirements (instead of Timer.cancel()).If you're not familiar with concurrent code check Java Concurrency in Practice.