Execute longtime threads in it's special order

Many time-consuming threads (500-900ms.) are created in the application. They are to be executed in the order they have been created - one after another - not simultaneously. The execution should be processed in thread, that is not synchronized with main application thread.

I can't make small threads executed in an order, so I found a ThreadPoolExecutor, but think it's too heavy for my task. So I wrote my Executor class.

It works fine. You add a thread to the threadList and it start the Executor thread to execute small tasks, that can be added while execution.

Can you tell me it's drawbacks and maybe another better way to solve my problem.

import java.util.LinkedList;
import java.util.List;

public class SfourExecutor extends Thread implements Runnable {

    private static List <Thread> threadList = new LinkedList<Thread>();
    private static final SfourExecutor INSTANCE = new SfourExecutor();

    public static List<Thread> getThreadList() {
        return threadList;
    }

    public static void setThreadList(List<Thread> threadList) {
        SfourExecutor.threadList = threa开发者_运维问答dList;
    }

    public void addToThreadList(Thread thread) {
        getThreadList().add(thread);
        if (!this.isAlive()) {
            this.start();
        }
    }

    public static SfourExecutor getInstance() {
        return SfourExecutor.INSTANCE;
    }

    private static class SfourHolder {
        private static final SfourExecutor INSTANCE = new SfourExecutor();
    }

    SfourExecutor () {
    }


    @Override
    public void run() {
        LinkedList <Thread> tL = (LinkedList<Thread>) getThreadList();

        while (!tL.isEmpty()) {
            Thread t = tL.poll();
            if (t!=null) {
                t.start();
                try {
                    t.join();
                } catch (InterruptedException ex) {
                }
            }
        }
    }

}

Won't an Executor created via Executors.newSingleThreadExecutor() meet your requirements exactly? "Tasks are guaranteed to execute sequentially, and no more than one task will be active at any given time."

Don't create new Thread instances when you don't need to execute jobs concurrently. Even if you are working with legacy code that has implemented some logic in a Thread class, you should be able to execute them as Runnable instances to the executor.

If you insist on using your own executor, you should know that your current implementation is not thread-safe. LinkedList is not a concurrent data structure, and since any code in the process can add new jobs at any time, you have no way to ensure that all jobs are added to the list before your executor thread is started. Thus, changes to the list are not guaranteed to be visible to all of the threads that are working with it.

Why are you using threads instead of Runnables? Isn't it easier to have a list of Runnables and call run() on them instead than a list of thread and wait for each one to end?

Apart from this, your solution seems fine.

In both cases, I would only add some form of synchronization on addToThreadList and around the block that checks if there are tasks to execute in the run() method, since there is a possibility that the two pieces of code execute at the same time on the same linked list, which is not synchronized.

Actually, you should really consider using an Executor. I don't think you'll find it heavy:

int maxThreads = 1;
ExecutorService myService = Executors.newFixedThreadPool(maxThreads);
...
myService.execute(myRunnable);

Always avoid rewriting something that exists already. You'll avoid writing new bugs. For instance, your implementation is not Thread safe. 2 simultaneous calls to addToThreadList expose you too an illegalthreadstateexception.

Nice code, but Executors.newFixedThreadPool(nThreads) nice too. If your threads make long work (500-900ms) in your tasks - pool overhead not affect performance. And JIT and another JVM runtime optimization can work better with standard classes.

You have a problem in that once the thread list is empty, your processing thread will stop checking for new threads added to the list. Also, LinkedLists aren't safe for concurrent usage, so what one thread submits might not be seen by the executor thread (or worse, it might not show up the way you submitted it). If you used a LinkedBlockingQueue it would work better.

However, since you want everything to run in sequence, you don't need to create and start lots of new threads (which is pretty costly), you can just have one thread that runs lightweight Runnable tasks in order. ThreadPoolExecutor does this for you, and more, and it is also really easy to use if you take help from the Executors class (which uses ThreadPoolExecutors behind the scenes):

ExecutorService executorService = Executors.newSingleThreadExecutor();
executorService.submit(new Runnable(){
    @Override
    public void run(){
        doSomethingThatTakesALongTime();
    }
});
executorService.submit(new Runnable(){
    @Override
    public void run(){
        doSomethingElseThatAlsoTakesALongTime();
    }
});

Concurrency isn't easy, so I'd advice you to make use of the fact that Sun Oracle have experts employed full-time to write and maintain these libraries for you. That means you get more free time you can spend at the pub, with your family, or implementing new features for your customers.