commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steven Sheehy (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (VALIDATOR-427) Race Condition in DomainValidator
Date Wed, 07 Jun 2017 16:16:18 GMT

    [ https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16041136#comment-16041136
] 

Steven Sheehy commented on VALIDATOR-427:
-----------------------------------------

My main focus is on the 4th bullet point above about not guaranteeing updateTLDOverride()
can be called first in all situations, so if we take the approach you recommend of setting
at construction time, that would work for me as well.

Though I believe that link you specified about volatile does not apply here since the arrays
are never modified after construction and are defensively copied when exposed. The volatile
keyword guarantees member assignment is atomic and, in this case, any use of that member is
read only and will at worse get an out of date object reference if updateTLDOverride() was
called. If need be, the internal array representation can be changed to a Collections.unmodifiableCollection()
to enforce immutability. I can submit a patch for this if you want? I'm not sure how to accomplish
the other construction-based you mention.

> Race Condition in DomainValidator
> ---------------------------------
>
>                 Key: VALIDATOR-427
>                 URL: https://issues.apache.org/jira/browse/VALIDATOR-427
>             Project: Commons Validator
>          Issue Type: Bug
>    Affects Versions: 1.6
>            Reporter: Steven Sheehy
>
> There's a race condition in DomainValidator which causes our application to fail sometimes.
The issue occurs when the DomainValidator.getInstance() is called before we can call DomainValidator.updateTLDOverride()
and we receive a IllegalStateException("Can only invoke this method before calling getInstance").
In a multi-threaded environment, DomainValidator.getInstance() can be called at any time and
it is difficult to find a location in application startup which ensures DomainValidator.updateTLDOverride()
is called before to initialize it. I was able to workaround during application runtime it
by placing the initialization in a Spring @Configuration class, but there is no proper location
in JUnit tests which can be called before any tests run.
> Therefore, I think the proper approach to address this is to allow DomainValidator.updateTLDOverride()
to be updated at any time including after calls to getInstance(). Examining the source, I
see that the both methods are synchronized and that the custom TLD arrays are all volatile.
Therefore, assuming Java 1.5 or greater and its guarantees about volatile assignments, the
code already guarantees proper synchronization for the TLD plus arrays and the inUse flag
is not needed and can be removed.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message