HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Delegating ExecutorService implementation requiring only an Executor

Submitted by: @import:stackexchange-codereview··
0
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:

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 synchronized do 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 CountDownLatch to support awaitTermination()




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.