New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using `@CacheNamespace` with `<cache-ref />` could cause exception #1194

Closed
abel533 opened this Issue Feb 24, 2018 · 8 comments

Comments

Projects
None yet
5 participants
@abel533
Copy link
Contributor

abel533 commented Feb 24, 2018

protected void buildAllStatements() {
if (!incompleteResultMaps.isEmpty()) {
synchronized (incompleteResultMaps) {
// This always throws a BuilderException.
incompleteResultMaps.iterator().next().resolve();
}
}
if (!incompleteCacheRefs.isEmpty()) {
synchronized (incompleteCacheRefs) {
// This always throws a BuilderException.
incompleteCacheRefs.iterator().next().resolveCacheRef();
}
}
if (!incompleteStatements.isEmpty()) {
synchronized (incompleteStatements) {
// This always throws a BuilderException.
incompleteStatements.iterator().next().parseStatementNode();
}
}
if (!incompleteMethods.isEmpty()) {
synchronized (incompleteMethods) {
// This always throws a BuilderException.
incompleteMethods.iterator().next().resolve();
}
}
}

when incompleteStatements is not empty and the parseStatementNode execute successful, incompleteStatements not remove it. it's also not empty.

Test

@CacheNamespace
public interface CountryCacheWithXmlMapper  {
    Country selectById(Integer id);
}
<!DOCTYPE mapper
        PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
        "http://mybatis.org/dtd/mybatis-3-mapper.dtd">

<mapper namespace="tk.mybatis.mapper.cache.CountryCacheWithXmlMapper">
    <!--这个例子是在接口中定义缓存,在 XML 中引用接口中配置的缓存-->
    <cache-ref namespace="tk.mybatis.mapper.cache.CountryCacheWithXmlMapper"/>

    <select id="selectById" resultType="tk.mybatis.mapper.base.Country">
        select * from country where id = #{id}
    </select>
</mapper>
@Test
public void testCountryCacheWithXmlMapper() {
    SqlSession sqlSession = getSqlSession();
    try {
        CountryCacheWithXmlMapper mapper = sqlSession.getMapper(CountryCacheWithXmlMapper.class);
        //This will throw an error
        Country country = mapper.selectById(35);
    } finally {
        sqlSession.close();
    }
}

Screenshots

invoke selectById:
why

configuration.hasStatement(statementId) will call buildAllStatements:
first

configuration.getMappedStatement(statementId) will call buildAllStatements:
second

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Feb 25, 2018

Hi @abel533 ,
Could you make it an executable test case including configurations, etc.?

@abel533

This comment has been minimized.

Copy link
Contributor Author

abel533 commented Feb 26, 2018

@harawata I uploaded an executable test case.

test-incomplete.zip

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Feb 26, 2018

Thank you, @abel533 !

I took a look and the problem, in this case, is the way you configured the mapper cache.
As mentioned in #452 , to apply cache on both XML and Java mapper, you need to specify <cache /> in XML mapper and reference it using @CacheNamespaceRef in Java mapper.

It would be nice if we could remove the restriction, of course.

@abel533

This comment has been minimized.

Copy link
Contributor Author

abel533 commented Feb 27, 2018

I can't believe I had mentioned it twice.

Although this may be limit, but buildAllStatements logic does have problem.

if (!incompleteStatements.isEmpty()) { 
 synchronized (incompleteStatements) { 
   // This always throws a BuilderException. 
   incompleteStatements.iterator().next().parseStatementNode(); 
 } 
} 

When will incompleteStatements be cleared?

@SinglerGodson

This comment has been minimized.

Copy link

SinglerGodson commented Jul 17, 2018

I have the same problem. I think incomplete statement should be removed after reparsed correctly. And also incompleteResultMaps, incompleteCacheRefs, incompleteMethods.
I found the remove operations in method XMLMapperBuiler.parsePendingStatements(), XMLMapperBuiler.parsePendingCacheRefs() and XMLMapperBuiler.parsePendingResultMaps().
So they should be removed here.

@gxing19

This comment has been minimized.

Copy link

gxing19 commented Sep 28, 2018

I`m using mybatis 3.4.6 release, this problem did not appear。 but In the 3.4.6 changes, this bug was not found to be fixed.
????

@lythesia

This comment has been minimized.

Copy link

lythesia commented Nov 8, 2018

@gxing19 Bug not fixed, it depends on the loading order & dependency of mappers, you might try some luck to reproduce it. Check #1176

@harawata harawata changed the title Configuration.buildAllStatements has problems Using `@CacheNamespace` with `<cache-ref />` could cause exception Nov 17, 2018

@harawata harawata self-assigned this Nov 17, 2018

@harawata harawata added this to the 3.5.0 milestone Nov 17, 2018

@harawata harawata closed this in 6fe5d62 Nov 17, 2018

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Nov 17, 2018

Hi guys,

Most problems reported with buildAllStatements() should be resolved in the latest 3.5.0-SNAPSHOT.
If there still is a similar problem (it's possible), please create a test case (see the ones I created) or post the full stack trace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment