[jira] [Commented] (VALIDATOR-427) Race Condition in DomainValidator
[
https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)
[jira] [Commented] (VALIDATOR-427) Race Condition in DomainValidator
[
https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16039720#comment-16039720
]
Sebb commented on VALIDATOR-427:
I suppose it might be possible to turn the override arrays into instance fields
set at construction.
This would allow an app to create mutiple validators.
That would be an enhancement request. It needs a fair amount of work to ensure
compatibility etc.
> 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)
[jira] [Commented] (VALIDATOR-427) Race Condition in DomainValidator
[
https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16039686#comment-16039686
]
Sebb commented on VALIDATOR-427:
The current rule is that the overrides must be set up before getting the
instance.
That was done in order to guarantee safe publication of the data without
requiring all the isValidxxx methods to be synchronised.
Volatile on its own does not do that, see:
https://www.securecoding.cert.org/confluence/display/java/CON50-J.+Do+not+assume+that+declaring+a+reference+volatile+guarantees+safe+publication+of+the+members+of+the+referenced+object
The inUse flag avoids the need to synch. the isValidxxx methods.
> 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)
[jira] [Commented] (VALIDATOR-427) Race Condition in DomainValidator
[
https://issues.apache.org/jira/browse/VALIDATOR-427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16039400#comment-16039400
]
Sebb commented on VALIDATOR-427:
There is no race condition here.
The rule is that DomainValidator.updateTLDOverride() must be called before
DomainValidator.getInstance().
If an application does not do that, it is an application bug.
The override arrays are static, and apply to all threads that use the singleton
instance.
If the overrides could be changed after getInstance has been called, then
different threads can get different results, possibly changing between calls.
That would be very confusing.
> 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)
