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, tasks that do not support interruption using {{Thread.interrupt()}} should not be submitted to a thread pool.  Similarly, when attempting to cancel individual tasks within the thread pool using the {{Future.cancel()}} method, ensure that the tasks support interruption. 


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 final Socket socket;
throws InterruptedException, // ...
}
{code}

Because the task does not support interruption through the use of {{Thread.interrupted()}}, there is no guarantee that the {{shutdownNow()}} method will quiickly 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 use some mechanism other than {{Thread.interrupted()}} to determine when to shutdown will be u nresponsive to {{shutdown()}} or {{shutdownNow()}}. For instance, tasks that check a {{volatile}} flag to determine whether it is safe to shutdown will be unresponsive to these methods. The guideline [CON13-J. Do not use Thread.stop() to terminate threads] provides more information on using a flag to terminate threads.


h2. Compliant Solution (submit interruptible tasks)

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 {
  // ...
}

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

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


h2. Exceptions

*EX1*: Tasks that execute without blocking may violate this guideline.

h2. Risk Assessment

Submitting tasks that are not interruptible may preclude the thread pool from shutting down 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!|CON35-J. Do not synchronize on the intrinsic locks of high-level concurrency objects]      [!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 {
  // ...
}