patternjavaMinor
Delegating ExecutorService implementation requiring only an Executor
Viewed 0 times
executorservicedelegatingexecutorimplementationonlyrequiring
Problem
I occasionally find myself needing to run code on some arbitrary thread or pass a Runnable to some arbitrary consumer, and missing the full functionality of java.util.concurrent -- for instance, writing Swing and JavaFX applications, posting UI updates from a background thread to the main UI thread with SwingUtilities.invokeLater() or Platform.runLater(). (I'm not an Android developer but I assume Android would be similar, with Activity.runOnUiThread().) It's easy enough to wrap these in an Executor:
However, that doesn't really get you much; in particular, it doesn't get you
It seems like there ought to be a library class out there that lets you bootstrap a simple Executor to a full ExecutorService—that this isn't the only situation where you might have some random thread you need to run things on and still want the full concurrency API. But I haven't been able to find it.
So here's what I cobbled together:
```
/**
* A basic ExecutorService that wraps any Executor.
*/
public class ExecutorExecutorService extends AbstractExecutorService {
// ------------------------------------------------------------
// Final fields
/** Main lock. */
private final Object mutex = new Object();
/** Underlying executor on which tasks are actualy run. */
private final Executor exec;
// ------------------------------------------------------------
// Mutable fields
/** Pending tasks in submission order. */
private final Set pending = new LinkedHashSet<>();
/** Currently running tasks. */
private final Set running = new HashSet<>();
/**
* Current run state.
*
* 2 - running (shutdown() not yet called)
* 1 - shut down, but not term
class AppThreadExecutor implements Executor {
@Override
public void execute(Runnable command) {
if (Platform.isFxApplicationThread()) {
command.run();
} else {
Platform.runLater(command);
}
}
}However, that doesn't really get you much; in particular, it doesn't get you
Futures, or any interaction with helpful APIs like those in Guava or Vavr.It seems like there ought to be a library class out there that lets you bootstrap a simple Executor to a full ExecutorService—that this isn't the only situation where you might have some random thread you need to run things on and still want the full concurrency API. But I haven't been able to find it.
So here's what I cobbled together:
```
/**
* A basic ExecutorService that wraps any Executor.
*/
public class ExecutorExecutorService extends AbstractExecutorService {
// ------------------------------------------------------------
// Final fields
/** Main lock. */
private final Object mutex = new Object();
/** Underlying executor on which tasks are actualy run. */
private final Executor exec;
// ------------------------------------------------------------
// Mutable fields
/** Pending tasks in submission order. */
private final Set pending = new LinkedHashSet<>();
/** Currently running tasks. */
private final Set running = new HashSet<>();
/**
* Current run state.
*
* 2 - running (shutdown() not yet called)
* 1 - shut down, but not term
Solution
- Does this look correct? Have I correctly understood the ExecutorService API?
I guess, so.
- Is it thread-safe? Are there any deadlocks or race conditions I haven't spotted?
It's probably fine. All your public methods either operate on thread-safe objects like
runState or synchronize on mutex.- Is it too thread-safe, i.e. am I synchronizing in places I don't have to?
All but one methods deal with shutdown, so synchronizing too much in them won't be a big deal. Anyway, I think, it's all necessary. You might be eliminate it by using concurrent sets.
Synchronizing inside of
execute feels somehow wrong as it might become a bottleneck in general (for things like Swing, it's probably fine). However, I can't see how to eliminate it.- Does
synchronizeddo the job here, or is there a reason to use a more sophisticated API?
I guess, it's fine as the only read operation need no lock (so
ReadWriteLock won't help) and you need no tryLock either.- Is there a better (= more legible, little or no more code) alternative to
CountDownLatchto supportawaitTermination()
I wondered why do you use it, but it seems to fit exactly.
This looks unnecessary complicated:
final Runnable[] pendingArray = pending.toArray(new Runnable[pending.size()]);
final List pendingList = Arrays.asList(pendingArray);
return Collections.unmodifiableList(pendingList);There's no double copying, but there should be a simpler way, even when you want to avoid dependency on Guava
return ImmutableList.copyOf(pending);Maybe just
return Collections.unmodifiableList(new ArrayList<>(pending));Summary
It looks good, but I'm afraid, I'm not sufficiently familiar with the executor stuff. I'm surprised that there's nothing like you did in either JDK nor Guava. You're upgrading an
Executor to an ExecutorService just like Guava MoreExecutors.listeningDecorator upgrades an ExecutorService to a ListeningExecutorService.Code Snippets
final Runnable[] pendingArray = pending.toArray(new Runnable[pending.size()]);
final List<Runnable> pendingList = Arrays.asList(pendingArray);
return Collections.unmodifiableList(pendingList);return ImmutableList.copyOf(pending);return Collections.unmodifiableList(new ArrayList<>(pending));Context
StackExchange Code Review Q#163003, answer score: 2
Revisions (0)
No revisions yet.