title: How to better use ExecutorService in multithreading environment?
body: <p>I need to make a library in which I will have synchronous and asynchronous methods in it.</p>
  
  <ul>
  <li><code>executeSynchronous()</code> - waits until I have a result, returns the result.</li>
  <li><code>executeAsynchronous()</code> - returns a Future immediately which can be processed after other things are done, if needed.</li>
  </ul>
  
  <p><strong>Core Logic of my Library</strong></p>
  
  <p>The customer will use our library and they will call it by passing <code>DataKey</code> builder object. We will then construct a URL by using that <code>DataKey</code> object and make a HTTP client call to that URL by executing it and after we get the response back as a JSON String, we will send that JSON String back to our customer as it is by creating <code>DataResponse</code> object. Some customer will call <code>executeSynchronous()</code> and some might call <code>executeAsynchronous()</code> so that's why I need to provide two method separately in my library.</p>
  
  <p>Interface:</p>
  
  <pre><code>public interface Client {
  
      // for synchronous
      public DataResponse executeSynchronous(DataKey key);
  
      // for asynchronous
      public Future&lt;DataResponse&gt; executeAsynchronous(DataKey key);
  }
  </code></pre>
  
  <p>And then I have my <code>DataClient</code> which implements the above <code>Client</code> interface:</p>
  
  <pre><code>public class DataClient implements Client {
  
      private RestTemplate restTemplate = new RestTemplate();
      // do I need to have all threads as non-daemon or I can have daemon thread for my use case?
      private ExecutorService executor = Executors.newFixedThreadPool(10);
  
      // for synchronous call
      @Override
      public DataResponse executeSynchronous(DataKey key) {
          DataResponse dataResponse = null;
          Future&lt;DataResponse&gt; future = null;
  
          try {
              future = executeAsynchronous(key);
              dataResponse = future.get(key.getTimeout(), TimeUnit.MILLISECONDS);
          } catch (TimeoutException ex) {
              PotoLogging.logErrors(ex, DataErrorEnum.TIMEOUT_ON_CLIENT, key);
              dataResponse = new DataResponse(null, DataErrorEnum.TIMEOUT_ON_CLIENT, DataStatusEnum.ERROR);
              future.cancel(true); // terminating tasks that have timed out
          } catch (Exception ex) {
              PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
              dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
          }
  
          return dataResponse;
      }
  
      //for asynchronous call
      @Override
      public Future&lt;DataResponse&gt; executeAsynchronous(DataKey key) {
          Future&lt;DataResponse&gt; future = null;
  
          try {
              Task task = new Task(key, restTemplate);
              future = executor.submit(task); 
          } catch (Exception ex) {
              PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
          }
  
          return future;
      }
  }
  </code></pre>
  
  <p>Simple class which will perform the actual task:</p>
  
  <pre><code>public class Task implements Callable&lt;DataResponse&gt; {
  
      private DataKey key;
      private RestTemplate restTemplate;
  
      public Task(DataKey key, RestTemplate restTemplate) {
          this.key = key;
          this.restTemplate = restTemplate;
      }
  
      @Override
      public DataResponse call() {
          DataResponse dataResponse = null;
          String response = null;
  
          try {
              String url = createURL();
              response = restTemplate.getForObject(url, String.class);
  
              // it is a successful response
              dataResponse = new DataResponse(response, DataErrorEnum.NONE, DataStatusEnum.SUCCESS);
          } catch (RestClientException ex) {
              PotoLogging.logErrors(ex, DataErrorEnum.SERVER_DOWN, key);
              dataResponse = new DataResponse(null, DataErrorEnum.SERVER_DOWN, DataStatusEnum.ERROR);
          } catch (Exception ex) { // should I catch RuntimeException or just Exception here?
              PotoLogging.logErrors(ex, DataErrorEnum.CLIENT_ERROR, key);
              dataResponse = new DataResponse(null, DataErrorEnum.CLIENT_ERROR, DataStatusEnum.ERROR);
          }
  
          return dataResponse;
      }
  
      // create a URL by using key object
      private String createURL() {
          String url = somecode;
          return url;
      }
  }
  </code></pre>
  
  <p>I have few questions on my above solution - </p>
  
  <ul>
  <li>Should I use daemon or non daemon threads for my above use case?</li>
  <li>Also, I am terminating the tasks that have timed out so that it doesn't occupy one of my limited 10 threads for a long time. Does that look right the way I am doing it?</li>
  <li>In my <code>call()</code> method, I am catching Exception. Should I catch <code>RuntimeException</code> there? What is the difference if I catch Exception or RuntimeException?</li>
  </ul>
  
  <p>When I started working on this solution, I was not terminating the tasks that have timed out. I was reporting the timeout to the client, but the task continues to run in the thread pool (potentially occupying one of my limited 10 threads for a long time). So I did some research online and I found that I can cancel my tasks those have timed out by using cancel on future as shown below -</p>
  
  <pre><code>future.cancel(true);
  </code></pre>
  
  <p>But I wanted to make sure, does it look right the way I am doing in my <code>executeSynchronous</code> method to cancel the tasks that have got timedout?</p>
  
  <p>Since I am calling <code>cancel()</code> on the <code>Future</code> which will stop it from running if tasks is still in the queue so I am not sure what I am doing is right or not? What is the right approach to do this?</p>
  
  <p>If there is any better way, then can anyone provide an example for that? </p>
  
  <p>Should we always be terminating the tasks that have got timed out? If we don't do that then what might be the impact I will have?</p>