TSM01-J
}
//Implementations of publishExceptionReporter(), setExceptionReporter() and
//report() are inherited
}
This approach ensures that the reporter cannot be set before the constructor has fully initialized the subclass and enabled logging.
6.2.7Noncompliant Code Example (Inner Class)
Inner classes maintain a copy of the this reference of the outer object. Consequently, the this reference may leak outside the scope [Goetz 2002]. This noncompliant code example uses a different implementation of the DefaultExceptionReporter class. The constructor uses an anonymous inner class to publish a filter() method.
public class DefaultExceptionReporter implements ExceptionReporter { public DefaultExceptionReporter(ExceptionReporter er) {
er.setExceptionReporter(new DefaultExceptionReporter(er) { public void report(Throwable t) {
filter(t);
}
});
}
// Default implementations of setExceptionReporter() and report()
}
The this reference of the outer class is published by the inner class so that other threads can see it. Furthermore, if the class is subclassed, the issue described in the noncompliant code example for handlers resurfaces.
6.2.8Compliant Solution
A private constructor alongside a public static factory method can safely publish the filter() method from within the constructor [Goetz 2006].
public class DefaultExceptionReporter implements ExceptionReporter { private final DefaultExceptionReporter defaultER;
private DefaultExceptionReporter(ExceptionReporter excr) { defaultER = new DefaultExceptionReporter(excr) {
public void report(Throwable t) { filter(t);
}
};
}
public static DefaultExceptionReporter newInstance(ExceptionReporter excr) { DefaultExceptionReporter der = new DefaultExceptionReporter(excr); excr.setExceptionReporter(der.defaultER);
return der;
CMU/SEI-2010-TR-015 | 154
TSM01-J
}
// Default implementations of setExceptionReporter() and report()
}
Because the constructor is private, untrusted code cannot create instances of the class, prohibiting the this reference from escaping. Using a public static factory method to create new instances also protects against publication of partially initialized objects (see guideline “TSM03-J. Do not publish partially initialized objects” on page 162) and untrusted manipulation of internal object state.
6.2.9Noncompliant Code Example (Thread)
This noncompliant code example starts a thread from within the constructor.
final class ThreadStarter implements Runnable { public ThreadStarter() {
Thread thread = new Thread(this); thread.start();
}
@Override public void run() { // ...
}
}
The new thread can access the this reference of the current object [Goetz 2002, Goetz 2006]. Notably, the Thread() constructor is alien to the ThreadStarter class.
6.2.10 Compliant Solution (Thread)
This compliant solution creates and starts the thread in a method instead of the constructor.
final class ThreadStarter implements Runnable { public ThreadStarter() {
// ...
}
public void startThread() {
Thread thread = new Thread(this); thread.start();
}
@Override public void run() { // ...
}
}
CMU/SEI-2010-TR-015 | 155
TSM01-J
6.2.11 Exceptions
TSM01-EX1: It is safe to create a thread in the constructor, provided the thread is not started until object construction has completed. This is because a call to start() on a thread happensbefore any actions in the started thread [Gosling 2005].
In this code example, even though a thread referencing this is created in the constructor, it is not started until its start() method is called from the startThread() method [Goetz 2002, Goetz 2006].
final class ThreadStarter implements Runnable { Thread thread;
public ThreadStarter() { thread = new Thread(this);
}
public void startThread() { thread.start();
}
@Override public void run() { // ...
}
}
TSM01-EX2: The ObjectPreserver pattern [Grand 2002] described in guideline “TSM02-J. Do not use background threads during class initialization” on page 157 is also a safe exception to this guideline.
6.2.12 Risk Assessment
Allowing the this reference to escape may result in improper initialization and runtime exceptions.
Guideline |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
|
|
|
|
|
|
TSM01-J |
medium |
probable |
high |
P4 |
L3 |
6.2.13 References
[Goetz 2002]
[Goetz 2006] |
Section 3.2, “Publication and Escape” |
[Gosling 2005] |
Keyword “this” |
[Grand 2002] |
Chapter 5, “Creational Patterns, Singleton” |
CMU/SEI-2010-TR-015 | 156
TSM02-J
6.3TSM02-J. Do not use background threads during class initialization
Starting and using background threads during class initialization can result in class initialization cycles and deadlock. For example, the main thread responsible for performing class initialization can block waiting for the background thread, which, in turn, will wait for the main thread to finish class initialization. This issue can arise, for example, when a database connection is established in a background thread during class initialization [Bloch 2005b].
6.3.1Noncompliant Code Example (Background Thread)
In this noncompliant code example, the static initializer starts a background thread as part of class initialization. The background thread attempts to initialize a database connection but needs to wait until all members of the ConnectionFactory class, including dbConnection, have been initialized.
public final class ConnectionFactory { private static Connection dbConnection; // Other fields ...
static {
Thread dbInitializerThread = new Thread(new Runnable() { @Override public void run() {
// Initialize the database connection try {
dbConnection = DriverManager.getConnection("connection string"); } catch (SQLException e) {
dbConnection = null;
}
}
});
// Other initialization, for example, start other threads
dbInitializerThread.start(); try {
dbInitializerThread.join();
}catch (InterruptedException ie) { throw new AssertionError(ie);
}
}
public static Connection getConnection() { if (dbConnection == null) {
throw new IllegalStateException("Error initializing connection");
}
return dbConnection;
}
public static void main(String[] args) {
CMU/SEI-2010-TR-015 | 157
TSM02-J
// ...
Connection connection = getConnection();
}
}
Statically initialized fields are guaranteed to be fully constructed before they are made visible to other threads. (See guideline “TSM03-J. Do not publish partially initialized objects” on page 162 for more information.) Consequently, the background thread must wait for the main (or foreground) thread to finish initialization before it can proceed. However, the ConnectionFactory class’s main thread invokes the join() method, which waits for the background thread to finish. This interdependency causes a class initialization cycle that results in a deadlock situation [Bloch 2005b].
Similarly, it is inappropriate to start threads from constructors. (See guideline “TSM01-J. Do not let the “this” reference escape during object construction” on page 149 for more information.) Creating timers that perform recurring tasks and starting those timers from within the code responsible for initialization introduces liveness issues.
6.3.2Compliant Solution (static Initializer, No Background Threads)
This compliant solution does not spawn any background threads from the static initializer. Instead, all fields are initialized in the main thread.
public final class ConnectionFactory { private static Connection dbConnection; // Other fields ...
static {
//Initialize a database connection try {
dbConnection = DriverManager.getConnection("connection string"); } catch (SQLException e) {
dbConnection = null;
}
//Other initialization (do not start any threads)
}
// ...
}
CMU/SEI-2010-TR-015 | 158
TSM02-J
6.3.3Compliant Solution (ThreadLocal)
This compliant solution initializes the database connection from a ThreadLocal object so that every thread can obtain its own instance of the connection.
public final class ConnectionFactory {
private static final ThreadLocal<Connection> connectionHolder
=new ThreadLocal<Connection>() {
@Override public Connection initialValue() { try {
Connection dbConnection = DriverManager.getConnection("connection string"); return dbConnection;
}catch (SQLException e) { return null;
}
}
};
// Other fields ...
static {
// Other initialization (do not start any threads)
}
public static Connection getConnection() { Connection connection = connectionHolder.get(); if (connection == null) {
throw new IllegalStateException("Error initializing connection");
}
return connection;
}
public static void main(String[] args) { // ...
Connection connection = getConnection();
}
}
The static initializer can be used to initialize any other shared class fields. Alternatively, the fields can be initialized from the initialValue() method.
CMU/SEI-2010-TR-015 | 159