Message Stores expected to be thread safe but are not

SessionState.java starts with the following bit of code:

/**

• Used by the session communications code. Not intended to be used by
• applications.
  *
• All dynamic data is protected by the session's intrinsic lock. The
• log and message store implementation must be thread safe.
  */
  public final class SessionState {
  private final Object lock;
  private final Log log;

// MessageStore implementation must be thread safe
private final MessageStore messageStore;
...
}

Yet, looking at the JdbcStore.java and MemoryStore.java implementations, they are in fact not thread safe as designed (may also be true for FileStore.java, but haven't checked).

The memory store uses a regular java.util.HashMap for all its operations, which is clearly not thread safe. I have never actually seen this cause a problem in practice, however.

The jdbc store does not synchronize its use of the jdbc connection, which can lead to race conditions inside the close() method of the jtds prepared statement, as shown in the stack trace below (specifically, the jtds close() method has a boolean to determine if its connection is already closed in which case it will not doubly close it despite any subsequent calls. This value is set once the close routine is complete but occurs after the connection object has already been assigned a null value- this allows another thread to potentially access the null connection object before the boolean guard has been set). Note that this bug occurs about once a week in a production environment (usually during startup), although its frequency is probably higher than average due to many quickfixj applications accessing the same database concurrently.

Example 1:

java.lang.NullPointerException
at net.sourceforge.jtds.jdbc.JtdsStatement.close(JtdsStatement.java:868)
at net.sourceforge.jtds.jdbc.JtdsPreparedStatement.close(JtdsPreparedStatement.java:485)
at org.logicalcobwebs.proxool.AbstractProxyStatement.close(AbstractProxyStatement.java:115)
at org.logicalcobwebs.proxool.ProxyStatement.invoke(ProxyStatement.java:93)
at org.logicalcobwebs.proxool.ProxyStatement.intercept(ProxyStatement.java:57)
at quickfix.JdbcUtil.close(JdbcUtil.java:143)
at quickfix.JdbcStore.storeSequenceNumbers(JdbcStore.java:288)
at quickfix.JdbcStore.setNextTargetMsgSeqNum(JdbcStore.java:272)
at quickfix.JdbcStore.incrNextTargetMsgSeqNum(JdbcStore.java:173)
at quickfix.SessionState.incrNextTargetMsgSeqNum(SessionState.java:359)
at quickfix.Session.nextLogon(Session.java:1535)
at quickfix.Session.next(Session.java:720)
at quickfix.mina.SingleThreadedEventHandlingStrategy$SessionMessageEvent.processMessage(SingleThreadedEventHandlingStrategy.java:106)
at quickfix.mina.SingleThreadedEventHandlingStrategy.block(SingleThreadedEventHandlingStrategy.java:70)
at quickfix.mina.SingleThreadedEventHandlingStrategy$1.run(SingleThreadedEventHandlingStrategy.java:86)
at java.lang.Thread.run(Thread.java:619)

Example 2:

java.lang.NullPointerException
at net.sourceforge.jtds.jdbc.JtdsStatement.close(JtdsStatement.java:868)
at net.sourceforge.jtds.jdbc.JtdsPreparedStatement.close(JtdsPreparedStatement.java:485)
at org.logicalcobwebs.proxool.AbstractProxyStatement.close(AbstractProxyStatement.java:115)
at org.logicalcobwebs.proxool.ProxyStatement.invoke(ProxyStatement.java:93)
at org.logicalcobwebs.proxool.ProxyStatement.intercept(ProxyStatement.java:57)
at quickfix.JdbcUtil.close(JdbcUtil.java:143)
at quickfix.JdbcStore.set(JdbcStore.java:259)
at quickfix.SessionState.set(SessionState.java:299)
at quickfix.Session.sendRaw(Session.java:1736)
at quickfix.Session.generateLogon(Session.java:1430)
at quickfix.Session.next(Session.java:1357)
at quickfix.mina.SessionConnector$SessionTimerTask.run(SessionConnector.java:248)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
at java.util.concurrent.FutureTask$Sync.innerRunAndReset(FutureTask.java:317)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:150)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:98)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.runPeriodic(ScheduledThreadPoolExecutor.java:181)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:205)
at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
at java.lang.Thread.run(Thread.java:619)