Potential race-condition in PojoTypes

[npetryk]

JDBI Version: 3.30.0

PojoTypes contains an internal map of factories which is copied in a number of situations, such as Jdbi.open(), or in my case when an on demand extension is used. This map is also modified by calls to register.

Today, for the first time that I've noticed, calling a @SqlBatch annotated method on an on-demand extension threw a mysterious ConcurrentModificationException, presumably because a factory was being registered at the same time that PojoTypes.factory was being copied (stacktrace below)

The good news is that subsequent calls appear to work just fine.

I haven't yet tried to repro this because I have a feeling it will be very tricky, and I wanted some validation from people more familiar with the guts of JDBI before embarking on that road.

Alternatively, since this error implies PojoTypes is not thread-safe, I would be happy to write up a PR making it thread-safe without repro'ing if we like the risk-to-reward ratio.

---

java.util.ConcurrentModificationException: null
at java.base/java.util.HashMap$HashIterator.nextNode(Unknown Source)
at java.base/java.util.HashMap$EntryIterator.next(Unknown Source)
at java.base/java.util.HashMap$EntryIterator.next(Unknown Source)
at java.base/java.util.HashMap.putMapEntries(Unknown Source)
at java.base/java.util.HashMap.putAll(Unknown Source)
at org.jdbi.v3.core.mapper.reflect.internal.PojoTypes.<init>(PojoTypes.java:32)
at org.jdbi.v3.core.mapper.reflect.internal.PojoTypes.createCopy(PojoTypes.java:52)
at org.jdbi.v3.core.mapper.reflect.internal.PojoTypes.createCopy(PojoTypes.java:25)
at org.jdbi.v3.core.config.ConfigRegistry.lambda$new$0(ConfigRegistry.java:58)
at java.base/java.util.concurrent.ConcurrentHashMap.forEach(Unknown Source)
at org.jdbi.v3.core.config.ConfigRegistry.<init>(ConfigRegistry.java:57)
at org.jdbi.v3.core.config.ConfigRegistry.createCopy(ConfigRegistry.java:121)
at org.jdbi.v3.sqlobject.SqlObjectFactory.attach(SqlObjectFactory.java:97)
at org.jdbi.v3.core.extension.Extensions.lambda$findFor$0(Extensions.java:75)
at java.base/java.util.Optional.map(Unknown Source)
at org.jdbi.v3.core.extension.Extensions.findFor(Extensions.java:75)
at org.jdbi.v3.core.Jdbi.callWithExtension(Jdbi.java:488)
at org.jdbi.v3.core.Jdbi.withExtension(Jdbi.java:478)
at org.jdbi.v3.core.internal.OnDemandExtensions.lambda$createProxy$3(OnDemandExtensions.java:82)
at jdk.proxy3/jdk.proxy3.$Proxy172.insert(Unknown Source)

... My Code ....
jdbi.onDemand(Dao.class).insert(...)

If I'm reading the following trace right, it implies that other.factories is being called concurrently - presumably because of a call to pojoTypes.register. I believe the bulk of my calls come from JdbiImmutables.registerImmutable and my framework ought to be making these calls during startup only, but I saw this exception regardless.

[stevenschlansker]

Thank you for reporting this - this definitely looks like a bug in Jdbi.

[hgschmie]

Are you using Jdbi with multiple threads / are you moving handles between threads? Do you use a shared Jdbi with multiple threads? I can see a number of situations where multiple threads step on each others' toes.

[npetryk]

Yes. The application is a micronaut web server, where a singleton Jdbi is injected into many @Singleton services, so request threads are sharing a single Jdbi instance. These singleton services call registerImmutable when they're constructed on application startup (though micronaut may be doing some things lazily)

We have multiple thread pools and we pass Handle through method calls to manage transactions, and use on-demand extensions when transactions are not important.

My understand is that both jdbi and handle are considered thread-safe

[npetryk]

I did a little more digging and I was able to confirm that my framework lazily initializes singletons. We do our registerImmutable calls at construction time of our data access services, so we very much do have registration calls concurrent with transactions & queries & things going on in other request threads.

A work around is to do all app initialization eagerly

[hgschmie]

Yes. The application is a micronaut web server, where a singleton Jdbi is injected into many @Singleton services, so request threads are sharing a single Jdbi instance. These singleton services call registerImmutable when they're constructed on application startup (though micronaut may be doing some things lazily)

We have multiple thread pools and we pass Handle through method calls to manage transactions, and use on-demand extensions when transactions are not important.

My understand is that both jdbi and handle are considered thread-safe

little late on this (I took a break from Jdbi), but I do want to point you at http://jdbi.org/#_handle. The long and the short is: If you create a Handle or a Query object on a thread and then pass it off to another thread for execution, you should be fine (there may be obscure bugs lurking but I am fairly certain that all the main paths and the error paths are good). If you hold on to a handle, execute it on one thread and then do some manipulation on another, you are almost guaranteed to run into threading issues.

[npetryk]

Thank you - very good to know