double-checked locking (BVAL-148)

[JB Nivoit <jn...@vmware.com>]

Hi all,

Maybe this belongs to the bval-dev mailing list, let me know.

In October 2016, there was a thread (see http://mail-archives.apache.org/mod_mbox//bval-dev/201610.mbox/thread) about issue BVAL-148 (direct link: https://issues.apache.org/jira/browse/BVAL-148?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel), and this issues was resolved as fixed. I believe it is in fact not fixed.
Making the “validator” data member in class ConstraintValidation volatile is enough to ensure that a concurrent reader sees the correct value for the reference, but it does not guarantee that the initialize() call on that object has completed in the thread that first detected the constraint validator to be null.
There is still a race condition here:

    public void validateGroupContext(final GroupValidationContext<?> context) {
        if (validator == null) {
            synchronized (this) {
                if (validator == null) {
                    try {
                        validator = getConstraintValidator(context.getConstraintValidatorFactory(), annotation,
                            validatorClasses, owner, access); ← on this line, because the reference has been set and is visible to other threads before initialize() is guaranteed to have completed.
                        if (validator != null) {
                            validator.initialize(annotation);
                        }


A fix is to remove the outer “if (validator == null) {“. Paying the code of synchronizing shouldn’t be expensive, and occurs at most once when the validator is being initialized…
I would love to see this fixed, as we ran into this bug in one of our products, and had to put in a work-around (since we do lazy initialization in our validator.initialize() method and see the isValid fail because it is called when the object isn’t initialized yet).

Thanks,

   Jean-Baptiste