AbstractJdbcCall's compiled variable should be declared as volatile [SPR-13617]

[spring-projects-issues]

Shimi Bandiel opened SPR-13617 and commented

AbstractJdbcCall contains a Double-Checked Locking code over the compiled field.
It should be declared volatile to avoid ordering issues that can result in race-condition scenarios.

---

Affects: 3.2.15, 4.1.8, 4.2.2

Referenced from: commits d42cc14, 81342f1, 0f4e4fc

Backported to: 4.1.9, 3.2.16

[spring-projects-issues]

Juergen Hoeller commented

Fair enough, we'll declare it as volatile.

However, please note that the effect is rather minimal: Since compilation is a one-time change in such an instance (i.e. an operation is never going to get un-compiled or re-compiled), other threads may miss an update to the compiled variable when it switches from false to true (which is an atomic operation with fully constructed state to be seen at any point). With checkCompiled(), this means it will unnecessarily invoke compile() - which checks the compiled flag again anyway, but within a synchronized block. So effectively, we optimize access via volatile to consistently avoid full synchronization...

Juergen

[spring-projects-issues]

Shimi Bandiel commented

I think that the really dangerous scenario can happen is the CPU will re-order the write to the compiled flag to a place before the 'compilation-logic' happens.
In that case another thread may use an non-compiled version of the object.

[spring-projects-issues]

Juergen Hoeller commented

Good point; I haven't seen this happening yet but it might theoretically occur.

The common version of the problem is delayed exposure of the new value to other threads since the CPU cache contains the stale old version still... even that is worth addressing since we don't want to enter full synchronization without a hard reason here.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

I've addressed the same problem in AbstractJdbcInsert and RdbmsOperation as well.

Juergen