Fix a race condition caused by other threads calling mapper methods while mappedStatements are being constructed

[tianshuang]

In projects using the Spring framework, we often use @PostConstruct to perform some operations after bean initialization. In some scenarios, we will create asynchronous thread to perform some time-consuming SQL operations to avoid blocking the application startup process. We observed that the use of mappers in asynchronous thread may broken due to the resize of the underlying StrictMap of mappedStatements.

Example code for using @PostConstruct with asynchronous thread, MybatisRaceConditionApplication.java:

@Autowired
private Mapper1 mapper1;

private volatile boolean applicationReady;

@PostConstruct
public void raceConditionTest() {
    new Thread(() -> {
        int successfulCalls = 0;
        while (!applicationReady) {
            try {
                mapper1.select1();
                successfulCalls++;
            } catch (Exception e) {
                System.err.println("Number of successful calls before application ready: " + successfulCalls);
                e.printStackTrace();
                return;
            }
        }
    }).start();
}

@EventListener(ApplicationReadyEvent.class)
public void doSomethingAfterStartup() {
    applicationReady = true;
    System.out.println("Context Ready Event received.");
}

In this scenario, the main thread is still building the mapper of other beans, that is, it keeps putting mappedStatement to mappedStatements, and in this asynchronous thread, it keeps getting mappedStatement from mappedStatements , because StrictMap inherits from HashMap, when the underlying HashMap performs the resize operation, before the old array element migration is completed and the asynchronous thread obtains the latest table reference, At this time, the mappedStatement that has been put before cannot be obtained from mappedStatements.

This is the source code of HashMap#resize in JDK8:

transient Node<K,V>[] table;

final Node<K,V>[] resize() {
    Node<K,V>[] oldTab = table;

    // omit newCap calculation

    Node<K,V>[] newTab = (Node<K,V>[])new Node[newCap];
    table = newTab;
    if (oldTab != null) {
        for (int j = 0; j < oldCap; ++j) {
            // omit elements reference copy
        }
    }
    return newTab;
}

In the source code, a new array newTab is allocated first, and the empty array newTab reference is assigned to table. Before the element migration is completed, the asynchronous thread calling mappedStatements.get(id) may not be able to get it mappedStatement(depends on seeing the latest table reference, note that table does not contain the volatile modifier. however, it can be obtained normally before and after resize), instead throws the following exception:

org.mybatis.spring.MyBatisSystemException: nested exception is org.apache.ibatis.exceptions.PersistenceException: 
### Error querying database.  Cause: java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1
### Cause: java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1
	at org.mybatis.spring.MyBatisExceptionTranslator.translateExceptionIfPossible(MyBatisExceptionTranslator.java:96)
	at org.mybatis.spring.SqlSessionTemplate$SqlSessionInterceptor.invoke(SqlSessionTemplate.java:441)
	at com.sun.proxy.$Proxy47.selectOne(Unknown Source)
	at org.mybatis.spring.SqlSessionTemplate.selectOne(SqlSessionTemplate.java:160)
	at org.apache.ibatis.binding.MapperMethod.execute(MapperMethod.java:87)
	at org.apache.ibatis.binding.MapperProxy$PlainMethodInvoker.invoke(MapperProxy.java:145)
	at org.apache.ibatis.binding.MapperProxy.invoke(MapperProxy.java:86)
	at com.sun.proxy.$Proxy50.select1(Unknown Source)
	at me.tianshuang.MybatisRaceConditionApplication.lambda$raceConditionTest$0(MybatisRaceConditionApplication.java:25)
	at java.lang.Thread.run(Thread.java:750)
Caused by: org.apache.ibatis.exceptions.PersistenceException: 
### Error querying database.  Cause: java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1
### Cause: java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1
	at org.apache.ibatis.exceptions.ExceptionFactory.wrapException(ExceptionFactory.java:30)
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:153)
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:145)
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:140)
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectOne(DefaultSqlSession.java:76)
	at sun.reflect.GeneratedMethodAccessor34.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.mybatis.spring.SqlSessionTemplate$SqlSessionInterceptor.invoke(SqlSessionTemplate.java:427)
	... 8 more
Caused by: java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1
	at org.apache.ibatis.session.Configuration$StrictMap.get(Configuration.java:1054)
	at org.apache.ibatis.session.Configuration.getMappedStatement(Configuration.java:844)
	at org.apache.ibatis.session.Configuration.getMappedStatement(Configuration.java:837)
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:150)
	... 15 more

The root cause of this problem is that multiple threads access mappedStatements concurrently, and the main thread has made structural modifications to mappedStatements, which does not conform to the usage specification of HashMap, so in order to solve this problem, we should make StrictMap inherited from ConcurrentHashMap, at the same time, in order to be compatible with the changes of the containsKey method and adapt to the NULL key, I rewrote the containsKey method.

This project can reproduce the race condition: GitHub - tianshuang/mybatis-race-condition

Reference:
java - HashMap resize() while single thread writing and multiple threads reading - Stack Overflow

Unit Test

All unit tests pass.

[harawata]

Hello @tianshuang ,

It seems like the same issue as mybatis/spring#696 which may not be resolved by this change.

we will create asynchronous thread to perform some time-consuming SQL operations to avoid blocking the application startup process.

I understand why people do this, but the current MyBatis design may not allow this approach (see my comments on the linked issue for why).

So, let me clarify. Did you verify that this change actually fixes your problem?
If you did, please let me know and I'll look into it.
But it may take some time as I don't have much spare time lately.

[tianshuang]

Yes, this PR fixed my problem, my problem is similar to mybatis/spring#696, but the problem I am facing has no dependencies between mappers, just because mappedStatements.get and mappedStatements.put are called concurrently by two threads, and at this time the underlying HashMap is executing the resize method. Note that exceptions only occur during resize.

As you say in mybatis/spring#696 (comment):

The fundamental difficulty is that there is no way to know if all mappers are loaded or not (note that addMapper could be called at any time, even from a user's code).

Since we can't prevent concurrent calls, why don't we make MyBatis more robust? This PR fixed the occasional exception: Mapped Statements collection does not contain value, details can be found here: GitHub - tianshuang/mybatis-race-condition. In this example, you can observe that the previously successfully executed mapper method suddenly throws java.lang.IllegalArgumentException: Mapped Statements collection does not contain value for me.tianshuang.mapper.Mapper1.select1, which is confusing. In fact, the underlying HashMap is being resize by other threads and the resize is not completed, resulting in the current thread unable to obtain the mapperStatement that can be obtained normally before. Focus on the following output from the example:

Number of successful calls before application ready: 1028910

[tianshuang]

Any ideas?

[coveralls]

Coverage increased (+0.01%) to 87.309% when pulling 7996f96 on tianshuang:race_condition into 043aaaa on mybatis:master.

[harawata]

@tianshuang ,

Thank you for the detailed explanation! I could verify the effect.
It should be included in the latest 3.5.12-SNAPSHOT