Is adding tasks to BlockingQueue of ThreadPoolExecutor advisable?

Question

The JavaDoc for ThreadPoolExecutor is unclear on whether it is acceptable to add tasks directly to the BlockingQueue backing the executor. The docs say calling executor.getQueue() is "intended primarily for debugging and monitoring".

I'm constructing a ThreadPoolExecutor with my own BlockingQueue. I retain a reference to the queue so I can add tasks to it directly. The same queue is returned by getQueue() so I assume the admonition in getQueue() applies to a reference to the backing queue acquired through my means.

Example

General pattern of the code is:

int n = ...; // number of threads queue = new ArrayBlockingQueue<Runnable>(queueSize); executor = new ThreadPoolExecutor(n, n, 1, TimeUnit.HOURS, queue); executor.prestartAllCoreThreads(); // ... while (...) {     Runnable job = ...;     queue.offer(job, 1, TimeUnit.HOURS); } while (jobsOutstanding.get() != 0) {     try {         Thread.sleep(...);     } catch (InterruptedException e) {         Thread.currentThread().interrupt();     } } executor.shutdownNow(); 

queue.offer() vs executor.execute()

As I understand it, the typical use is to add tasks via executor.execute(). The approach in my example above has the benefit of blocking on the queue whereas execute() fails immediately if the queue is full and rejects my task. I also like that submitting jobs interacts with a blocking queue; this feels more "pure" producer-consumer to me.

An implication of adding tasks to the queue directly: I must call prestartAllCoreThreads() otherwise no worker threads are running. Assuming no other interactions with the executor, nothing will be monitoring the queue (examination of ThreadPoolExecutor source confirms this). This also implies for direct enqueuing that the ThreadPoolExecutor must additionally be configured for > 0 core threads and mustn't be configured to allow core threads to timeout.

tl;dr

Given a ThreadPoolExecutor configured as follows:

• core threads > 0
• core threads aren't allowed to timeout
• core threads are prestarted
• hold a reference to the BlockingQueue backing the executor

Is it acceptable to add tasks directly to the queue instead of calling executor.execute()?

Related

This question ( producer/consumer work queues ) is similar, but doesn't specifically cover adding to the queue directly.

Answer 1

One trick is to implement a custom subclass of ArrayBlockingQueue and to override the offer() method to call your blocking version, then you can still use the normal code path.

queue = new ArrayBlockingQueue<Runnable>(queueSize) {   @Override public boolean offer(Runnable runnable) {     try {       return offer(runnable, 1, TimeUnit.HOURS);     } catch(InterruptedException e) {       // return interrupt status to caller       Thread.currentThread().interrupt();     }     return false;   } }; 

(as you can probably guess, i think calling offer directly on the queue as your normal code path is probably a bad idea).

Answer 2

If it were me, I would prefer using Executor#execute() over Queue#offer(), simply because I'm using everything else from java.util.concurrent already.

Your question is a good one, and it piqued my interest, so I took a look at the source for ThreadPoolExecutor#execute():

public void execute(Runnable command) {     if (command == null)         throw new NullPointerException();     if (poolSize >= corePoolSize || !addIfUnderCorePoolSize(command)) {         if (runState == RUNNING && workQueue.offer(command)) {             if (runState != RUNNING || poolSize == 0)                 ensureQueuedTaskHandled(command);         }         else if (!addIfUnderMaximumPoolSize(command))             reject(command); // is shutdown or saturated     } } 

We can see that execute itself calls offer() on the work queue, but not before doing some nice, tasty pool manipulations if necessary. For that reason, I'd think that it'd be advisable to use execute(); not using it may (although I don't know for certain) cause the pool to operate in a non-optimal way. However, I don't think that using offer() will break the executor - it looks like tasks are pulled off the queue using the following (also from ThreadPoolExecutor):

Runnable getTask() {     for (;;) {         try {             int state = runState;             if (state > SHUTDOWN)                 return null;             Runnable r;             if (state == SHUTDOWN)  // Help drain queue                 r = workQueue.poll();             else if (poolSize > corePoolSize || allowCoreThreadTimeOut)                 r = workQueue.poll(keepAliveTime, TimeUnit.NANOSECONDS);             else                 r = workQueue.take();             if (r != null)                 return r;             if (workerCanExit()) {                 if (runState >= SHUTDOWN) // Wake up others                     interruptIdleWorkers();                 return null;             }             // Else retry         } catch (InterruptedException ie) {             // On interruption, re-check runState         }     } } 

This getTask() method is just called from within a loop, so if the executor's not shutting down, it'd block until a new task was given to the queue (regardless of from where it came from).

Note: Even though I've posted code snippets from source here, we can't rely on them for a definitive answer - we should only be coding to the API. We don't know how the implementation of execute() will change over time.