0

I want to write a reusable piece of code to allow waiting conditions while submitting tasks to an executor service. There are alot of implementaions for neat ways of blocking if too many tasks are queue, e.g. here

I need a executor that evaluates all waiting threads, every time on task is finished. For deciding if task is allowed to be submitted atm, the current state of all active tasks must be considered. I came up with the following solution, which doesn't have to scale for multiple submitters or a high grade of simultaneous executed tasks.

Question: Is the following code safe to use, or is there some flaw that I'm missing? The person implementing the aquireAccess method of the ConditionEvaluator<T> must ensure that the way the state of the threads in queried is thread safe, but the implementer needn't safeguard the iteration over the activeTasks collection. Here is the code:

public class BlockingExecutor<T extends Runnable> {

    private final Executor executor;

    private final ConditionEvaluator<T> evaluator;

    final ReentrantLock lock = new ReentrantLock();

    final Condition condition = this.lock.newCondition();

    final LinkedList<T> active = new LinkedList<T>();

    private final long reevaluateTime;

    private final TimeUnit reevaluateTimeUnit;

    public BlockingExecutor(Executor executor, ConditionEvaluator<T> evaluator) {
        this.evaluator = evaluator;
        this.executor = executor;
        this.reevaluateTimeUnit = null;
        this.reevaluateTime = 0;
    }

    public BlockingExecutor(Executor executor, ConditionEvaluator<T> evaluator, long reevaluateTime, TimeUnit reevaluateTimeUnit) {
        this.evaluator = evaluator;
        this.executor = executor;
        this.reevaluateTime = reevaluateTime;
        this.reevaluateTimeUnit = reevaluateTimeUnit;
    }

    public void submitTask(final T task) throws InterruptedException {
        this.lock.lock();
        try {
            do{
            if (this.reevaluateTimeUnit == null) {
                this.condition.await(this.reevaluateTime, this.reevaluateTimeUnit);
            } else {
                this.condition.await();
            }
            }while(!this.evaluator.aquireAccess(this.active, task));
                this.active.add(task);
                this.executor.execute(new Runnable() {

                    @Override
                    public void run() {
                        try {
                            task.run();
                        } finally {
                            BlockingExecutor.this.lock.lock();
                            try{
                                BlockingExecutor.this.active.remove(task);
                                BlockingExecutor.this.condition.signalAll();
                            }finally{
                                BlockingExecutor.this.lock.unlock();
                            }

                        }
                    }
                });
        } finally {
            this.lock.unlock();
        }
    }
}

public interface ConditionEvaluator<T extends Runnable> {
    public boolean aquireAccess(Collection<T> activeList,T task);   
}

Question: Can the code be improved?

Gray
  • 115,027
  • 24
  • 293
  • 354
Franz Kafka
  • 10,623
  • 20
  • 93
  • 149

0 Answers0