[ 
https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Steven Sheehy reopened VALIDATOR-427:
-------------------------------------

I respectfully disagree and would appreciate some discussion on it before 
dismissing it out of hand.

- This issue ended up causing a NoClassDefFoundError in production since we 
were initializing updateTLDOverride in a static initializer and some other code 
was calling getInstance() before that code was called. This is a serious issue 
that deserves some thought.

- To your point about confusion after changing them, there would be no 
confusion since the developer is the one that changed that and would expect the 
result to change.

- What about the use case where the end user would want to add some custom 
domains dynamically via a UI? We have large enterprise customers who pick 
non-standard local TLDs and right now we're having to hardcode them.

- There are simply times when we cannot guarantee updateTLDOverride() is called 
first. Where would you suggest we call updateTLDOverride() to ensure that it is 
called first in a series of JUnit tests? The order JUnit runs tests is random 
and there is no concept of "execute this code before all tests" in JUnit. Or 
what about if a library I wrote depended upon updateTLDOverride() and had 
multiple classes that served as entry points, where would I initialize it there?

- The inUse flag serves no synchronization purpose, only an arbitrary technical 
requirement that TLDs can never be set after getInstance().

> 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)

Reply via email to