Follow up of this question.Things altered: Put logging (debug level) for creating and starting thread.A separate thread for starting the different threads.Locking object and synchronise for starting the threadIncreaser.Changed the while condition of the while loop of threadIncreaser so getting the size of the queue only happens when we don't have the maximum threads running.Changed run to runningRemoved the interface runnable.public enum MailQueue {    INSTANCE;    private JavaMailSender sender;    private boolean running = false;    private final Thread threadIncreaser = new Thread(new Runnable() {        @Override        public void run() {            LOGGER.debug(ThreadIncreaser started);            int currentThreads = CURRENT_THREADS_SEND_MAIL.get();            while (currentThreads < MAX_THREADS_SEND_MAIL && mailsToSend.size() > (MAX_ELEMENTS_BEFORE_NEW_THREAD * currentThreads)) {                new Thread(createSendMailsThread(currentThreads + 1)).start();                currentThreads = CURRENT_THREADS_SEND_MAIL.incrementAndGet();                LOGGER.debug((Thread  + currentThreads +  created));            }            threadIncreaserRunning = false;        }    });    private boolean threadIncreaserRunning = false;    private static final Object THREAD_INCREASER_LOCK_OBJECT = new Object();    private final ConcurrentLinkedQueue<MimeMessage> mailsToSend = new ConcurrentLinkedQueue<MimeMessage>();    private final ConcurrentLinkedQueue<MimeMessage> errorRun = new ConcurrentLinkedQueue<MimeMessage>();    private final Map<MimeMessage, MailException> mailsWithErrors = new ConcurrentHashMap<MimeMessage, MailException>();    private static final Logger LOGGER = LoggerFactory.getLogger(MailQueue.class);    private static final int WAIT_FAILURE_TIME = 120000;    private static final int MAX_THREADS_SEND_MAIL = 4;    private static final int MAX_ELEMENTS_BEFORE_NEW_THREAD = 25;    private static final AtomicInteger CURRENT_THREADS_SEND_MAIL = new AtomicInteger(0);    /**     * Adding a mail to the Queue. When Queue is not started, it will start.     *     * @param message to send.     * @return true is mail is successfully added to the Queue     */    public boolean addMail(MimeMessage message) {        boolean result = mailsToSend.add(message);        checkAliveAndStartThreadCounts();        return result;    }    /**     * Adding a mail to the Queue. When Queue is not started, it will start.     *     * @param messages to send.     * @return true is mail is successfully added to the Queue     */    public boolean addMails(Set<MimeMessage> messages) {        boolean result = mailsToSend.addAll(messages);        checkAliveAndStartThreadCounts();        return result;    }    /**     *     * @param threadNumber     * @return the Thread for sending mails.     */    private Thread createSendMailsThread(final int threadNumber) {        return new Thread(new Runnable() {            @Override            public void run() {                LOGGER.debug(Thread  + threadNumber +  started);                running = true;                while (mailsToSend.peek() != null) {                    MimeMessage message = mailsToSend.remove();                    sendMessage(message);                }                running = false;                if (CURRENT_THREADS_SEND_MAIL.decrementAndGet() < 1) {                    getErrorThread().start();                }            }        });    }    /**     * Removes a specific mail from the error list.     *     * @param message to remove     * @throws MessagingException When there is a fault with getting recipients     * for logging. Mail is NOT removed when this error comes up.     */    public void removeMailFromError(MimeMessage message) throws MessagingException {        LOGGER.info(Removed mail to  + message.getRecipients(Message.RecipientType.TO)[0].toString()                + \nWith title :  + message.getSubject() + from error queue. \nError was :  + mailsWithErrors.remove(message).getMessage());    }    /**     * Starts a new Thread, to try sending the erroneous mails again.     */    public void startErrorThread() {        LOGGER.debug(Creating error Thread);        getErrorThread().start();    }    /**     * Try to send this specific mail from error list.     *     * @param message to send     * @return True if mail was send.     */    public boolean trySingleErrorMail(MimeMessage message) {        if (sendMessage(message)) {            LOGGER.trace(erroneous mail succesfull send, mailsWithErrors.remove(message));            return true;        }        return false;    }    /**     * Check if the threadincreaser is dead, if so => start it over.     */    private void checkAliveAndStartThreadCounts() {        synchronized (THREAD_INCREASER_LOCK_OBJECT) {            if (!threadIncreaserRunning) {                threadIncreaserRunning = true;                LOGGER.debug(Starting the threadIncreaser);                threadIncreaser.start();            }        }    }    /**     * Creates a Thread for sending all the erroneous mails again.     *     * @return the Error thread     */    private Thread getErrorThread() {        return new Thread(new Runnable() {            @Override            public void run() {                LOGGER.debug(Error thread started);                pauze(WAIT_FAILURE_TIME);                tryErrorsAgain();            }            private void pauze(int time) {                try {                    Thread.sleep(time);                } catch (InterruptedException ex) {                    LOGGER.error(sleep interrupted., ex);                }            }        });    }    private void tryErrorsAgain() {        errorRun.addAll(mailsWithErrors.keySet());        while (errorRun.peek() != null) {            MimeMessage message = errorRun.remove();            if (sendMessage(message)) {                MailException exception = mailsWithErrors.remove(message);                if (exception != null) {                    LOGGER.trace(Errorneous mail succesfull send., exception);                }            }        }    }    private boolean sendMessage(MimeMessage message) {        MailException exception;        try {            sender.send(message);            return true;        } catch (MailException e) {            try {                LOGGER.error(sending mail failed  + String.valueOf(message.getRecipients(Message.RecipientType.TO)[0]), e);            } catch (MessagingException ex) {                LOGGER.error(This error shouldn't happen., ex);            }            exception = mailsWithErrors.put(message, e);            if (exception != null) {                LOGGER.trace(Added duplicated mail in errors, e);            }        }        return false;    }    public MimeMessage createMimeMessage() {        return sender.createMimeMessage();    }    public void setSender(JavaMailSender sender) {        this.sender = sender;    }    public Map<MimeMessage, MailException> getMailsWithErrors() {        return mailsWithErrors;    }    public Collection<MimeMessage> getToSend() {        return Collections.unmodifiableList(Arrays.asList(mailsToSend.toArray(new MimeMessage[0])));    }    public boolean isRunning() {        return running;    }}
MailQueue - follow up
java;multithreading;queue;email
In some cases this is a reiteration of the review you were given on the original question, some parts I found myself.As Pimgd already stated in his review:Do it the proper way. Have one class that keeps track of the tasks and  one class that does the tasks. Not this self-forking madness where you  keep a reference to the main instance by a enum variable.He's completely right. Your design for this class is madness. What you seem to be looking for is a Facade-Structure. What you should do to accomplish that is: Create an interface, exposing the methods you need, and start from there.public interface MailQueue {    boolean addMail(MimeMessage message);    boolean addMails(Collection<MimeMessage> messages);    void resendErroredMails();    void retrySingleErroredMail(MimeMessage message);    boolean isRunning();    MimeMessage createMimeMessage();}I left out some of the public methods and slightly changed a few others. For one: You shouldn't expose the internal workings of your MailQueue, and you also probably shouldn't allow externals to change it. But that's what you do in: getMailsWithError, setSender (and also getSender), startErrorThread, removeMailFromError.getMailsWithError should return an immutable Collection<MimeMessage>, if you want to implement it. I seriously doubt the need for it, though.The JavaMailSender you're using is begging to be made final, since it shouldn't need to change. Programming against Threads is hard. As Eric Lippert stated in a comment on his blog:Many people think of threads as units of work, but they are not. Threads are workers. Most of the problems you see in multithreaded systems have analogous problems in single-threaded systems. People are just not yet in the habit of mentally separating workers from work.What does that mean for you? Well... Use existing solutions for the Problem of I have work I need done multithreaded, namely a ThreadPoolExecutor, because that thing is what you tried to reinvent here. The whole Thread limit and more threads for more work problematic can be solved using that thing.In the end the functionality you have boils down to:accept data to sendtry sending itif sending it fails    put it into a collection that can be scheduled for resendingWhat you need for that is a single BlockingQueue, the JavaMailSender and a ThreadPoolExecutor. If you want to be fancy you can make the resending asynchronous.