Page History

public final class SocketReader implements Runnable { // Thread-safe class
  private final Socket socket;
  private final BufferedReader in;
  private final Object lock = new Object();

  public SocketReader(String host, int port) throws IOException {
    this.socket = new Socket(host, port);
    this.in = new BufferedReader(
        new InputStreamReader(this.socket.getInputStream())
    );
  }

  // Only one thread can use the socket at a particular time
  @Override public void run() {
    try {
      synchronized (lock) {
        readData();
      }
    } catch (IOException ie) {
      // Forward to handler
    }
  }

  public void readData() throws IOException {
    String string;
    try {
      while ((string = in.readLine()) != null) {
        // Blocks until end of stream (null)
      }
    } finally {
      shutdown();
    }
  }

  public void shutdown() throws IOException {
    socket.close();
  }
}
 terminate. 
{quote}

Consequently, care must be taken so that only interruptible tasks are submitted to the thread pool. When using worker threads that run untrusted code or dynamically loaded extension code, a way to interrupt the worker must be provided.  For instance, a worker thread in a thread pool should generally have the following structure (adopted from \[[Goetz 06|AA. Java References#Goetz 06]\]):

{code:bgColor=#ccccff}
public void run() {
  Throwable thrown = null;
  try {
    while (!isInterrupted())
      runTask(getTaskFromWorkQueue());
  } catch (Throwable e) {
    thrown = e;
  } finally {
    threadExited(this, thrown); // Notify upper layers
  }
}
{code} 

The worker thread based architecture that uses interruptible workers permits the caller to cleanly terminate misbehaving sub-tasks. If the sub-tasks die because of runtime exceptions, then the upper layers can take some suitable action such as replacing the worker thread with a new one.


h2. Noncompliant Code Example (shutting down thread pools)

This noncompliant code example uses the {{SocketReader}} class defined earlier in the Compliant Solution (close socket connection) of the guideline [CON24-J. Ensure that threads and tasks performing blocking operations can be terminated] and submits it as a task to a thread pool defined in class {{PoolService}}.  

{code:bgColor=#FFcccc}
public final class PoolService {
  private final ExecutorService pool;

  public PoolService(int poolSize) {
    pool = Executors.newFixedThreadPool(poolSize);
  }
	  
  public void doSomething() throws InterruptedException, IOException {	   
    pool.submit(new SocketReader("somehost", 8080));
    // ...
    List<Runnable> awaitingTasks = pool.shutdownNow();	      
  }

  public static void main(String[] args) throws
 InterruptedException, IOException {
    PoolService service = new PoolService(5);
    service.doSomething();
  }
}

public final class SocketReader implements Runnable {
  private finalthrows Socket socket;
  // ...
}
{code}

Because the task does not support interruption through the use of {{Thread.interrupted()}}, there is no guarantee that the {{shutdownNow()}} method will shutdown the thread pool. Using the {{shutdown()}} method does not fix the problem either, because it waits until all executing tasks have finished. {mc} I think the general policy is to avoid "will" {mc} Similarly, tasks that check a {{volatile}} flag to determine whether it is safe to shutdown will also be unresponsive to these methods. The guideline [CON13-J. Ensure that threads are stopped cleanly] provides more information on using a flag to terminate threads. {mc} I added a reference instead of explaining {mc} 

h2. Compliant Solution (submit interruptible tasks)

Tasks that do not support interruption using {{Thread.interrupt()}} should not be submitted to a thread pool. This compliant solution submits the interruptible version of {{SocketReader}} discussed in the Compliant Solution (interruptible channel) of the guideline [CON24-J. Ensure that threads and tasks performing blocking operations can be terminated], to the thread pool.  

{code:bgColor=#ccccff}
public final class PoolService {
  // ...
}

InterruptedException, IOException {
    PoolService service = new PoolService(5);
    service.doSomething();
  }
}

public final class SocketReader implements Runnable {
  private final SocketChannel sc;
  // ...
}
{code}

{mc} Can we avoid "you" to be more formal and consistent with other guidelines?
Similarly, when attempting to cancel individual tasks within the thread pool using the {{Future.cancel()}} method, you must ensure that the tasks support interruption. If they do, then you must pass a {{boolean}} argument {{true}} to {{cancel()}}, otherwise you must pass {{false}}. The value {{false}} indicates that the corresponding task will be canceled if it has not already started.
{mc}

Similarly, when attempting to cancel individual tasks within the thread pool using the {{Future.cancel()}} method, ensure that the tasks support interruption. If they do, pass a {{boolean}} argument {{true}} to {{cancel()}}, otherwise pass {{false}}. The value {{false}} indicates that the corresponding task will be canceled if it has not already started.


h2. Exceptions

*EX1*: Tasks that execute quickly and do not block may violate this guideline.

h2. Risk Assessment

Submitting tasks that are not interruptible may preclude the shut down procedure of a thread pool and cause denial of service.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON36- J | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} |



h3. Automated Detection

TODO



h3. Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON35-J].

h2. References

\[[API 06|AA. Java References#API 06]\] interface ExecutorService
\[[Goetz 06|AA. Java References#Goetz 06]\] Chapter 7: Cancellation and shutdown

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|CON34-J. Do not use an instance lock to protect shared static data]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON37-J. Ensure that tasks executing in a thread pool do not fail silently]

private final Object lock = new Object();

  public SocketReader(String host, int port) throws IOException {
    sc = SocketChannel.open(new InetSocketAddress(host, port));
  }

  @Override public void run() {
    ByteBuffer buf = ByteBuffer.allocate(1024);
    try {
      synchronized (lock) {
        while (!Thread.interrupted()) {
          sc.read(buf);
          // ...
        }
      }
    } catch (IOException ie) {
      // Forward to handler
    }
  }
}

public final class PoolService {
  // ...
}