The interceptor org.apache.tomcat.jdbc.pool.interceptor.StatementFinalizer is not thread-safe. Some JDBC drivers (e.g. MSSQL) allow to use a single JDBC connection in multiple threads (a statement cannot be shared across threads). In our application we use multiple different threads which operate on the same JDBC connection. If a statement is created then the StatementFinalizer stores it in a java.util.LinkedList (which is not thread-safe and may damage the data structure). This might lead to a java.lang.NullPointerException at java.util.LinkedList.unlink(LinkedList.java:211) at java.util.LinkedList.remove(LinkedList.java:526) at org.apache.tomcat.jdbc.pool.interceptor.StatementFinalizer.closeInvoked(StatementFinalizer.java:59) at org.apache.tomcat.jdbc.pool.interceptor.AbstractCreateStatementInterceptor.invoke(AbstractCreateStatementInterceptor.java:59) at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:108) at org.apache.tomcat.jdbc.pool.interceptor.ConnectionState.invoke(ConnectionState.java:152) at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:108) at org.apache.tomcat.jdbc.pool.TrapException.invoke(TrapException.java:40) Our workaround was to create our own thread-safe statement finalizer.
The JDBC spec doesn't mandate thread-safety for the various classes, so you shouldn't expect them to be thread-safe. That said, it seems like StatementFinalizer should probably be thread-safe in case the JDBC driver itself guarantees thread safety. Or, even better, a MT-safe StatementFinalizer could be provided when it is required (which I would think would be an unusual situation). Would you care to donate your MT-safe StatementFinalizer class to the project?
Created attachment 34679 [details] A thread-safe variant of the original StatementFinalizer This is a slightly modified version of the original StatementFinalizer. There are only two changes: 1) It uses an ArrayList instead of a LinkedList. 2) The access on the "statements" list is synchronized. Note that I don't think a more sophisticated implementation (e.g. with a concurrent copy-on-write datastructure) is needed because I'd not expect a high thread contention.
Could this class have been written as a subclass of StatementFinalizer that simply overrides all the methods with synchronized versions and delegates to the superclass? That would be less code to maintain.
(In reply to Christopher Schultz from comment #3) > Could this class have been written as a subclass of StatementFinalizer that > simply overrides all the methods with synchronized versions and delegates to > the superclass? That would be less code to maintain. The new StatementFinalizer class also replaces the LinkedList with an ArrayList (but this is not the important change - I think it would also be okay if you would keep the original LinkedList implementation). But why would you want to keep both the original StatementFinalizer class and the thread-safe StatementFinalizer class? I think it would be really good to have just one StatementFinalizer class which is thread-safe.
(In reply to baier from comment #4) > But why would you want to keep both the original StatementFinalizer class > and the thread-safe StatementFinalizer class? Synchronization is not necessary for most users, so they don't need to pay the penalty. Yes, uncontested locks are fairly inexpensive, but when they aren't needed at all, they can be completely eliminated.