Actually, thinking about this some more, that will only partially fix
the problem. The real fix is to make each of the methods getConj,
getPreps, getPrepsC look like this:
public static GrammaticalRelation getConj(String conjunctionString) {
GrammaticalRelation result = conjs.get(conjunctionString);
if (result == null) {
synchronized(conjs) {
result = conjs.get(conjunctionString);
if (result == null) {
result = new GrammaticalRelation(Language.English, "conj",
"conj_collapsed", null, CONJUNCT, conjunctionString);
conjs.put(conjunctionString, result);
threadSafeAddRelation(result);
}
}
}
return result;
}
and the maps need to be declared volatile
(http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html)
Otherwise, we will wind up adding multiple of the same relation to
conjs, preps, prepsc in certain concurrency situations. Any further
thoughts from anyone else on improving this are welcome, of course.
John
On Thu, Jan 26, 2012 at 6:14 PM, John Bauer <horatio at gmail.com> wrote:
> I think we're well past the point where we can sneak fixes in to the
> currently available source. (For one thing, we pushed it to Maven
> already...) However, I think the following changes might fix this
> situation. There are three Map<...> structures in
> EnglishGrammaticalRelations, each of which need to be changed from
> Generics.newHashMap() to Generics.newConcurrentHashMap().
>> Would you let us know if that works?
>> Thanks,
>> John
>> On Mon, Jan 23, 2012 at 6:27 AM, Ioan Barbulescu <ibarbulescu at gmail.com> wrote:
>>>> Dear John & listers
>>>> This is a brief follow-up on our previous discussion about thread-safe
>> quality of the GrammaticalStructure.
>>>> The fix made by John (mentioned below) has indeed fixed the original error.
>>>> However, I am now getting a similar error, in a different place of the
>> EnglishGrammaticalStructure:
>>>> java.util.ConcurrentModificationException
>> at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
>> at java.util.HashMap$ValueIterator.next(HashMap.java:835)
>> at java.util.AbstractCollection.addAll(AbstractCollection.java:333)
>> at java.util.HashSet.<init>(HashSet.java:117)
>> at
>> edu.stanford.nlp.trees.EnglishGrammaticalStructure.removeDep(EnglishGrammaticalStructure.java:1752)
>> at
>> edu.stanford.nlp.trees.EnglishGrammaticalStructure.collapseDependencies(EnglishGrammaticalStructure.java:223)
>> at
>> edu.stanford.nlp.trees.GrammaticalStructure.typedDependenciesCollapsed(GrammaticalStructure.java:642)
>> at
>> edu.stanford.nlp.trees.GrammaticalStructure.typedDependenciesCollapsed(GrammaticalStructure.java:612)
>>>> Can you take a quick look at this new Exception?
>>>>>> Many thanks.
>>>> Best regards,
>> Ioan
>>>>>> On Thu, Dec 22, 2011 at 11:20 AM, Ioan Barbulescu <ibarbulescu at gmail.com>
>> wrote:
>>>>>> Dear John
>>>>>> I confirm that changing the last parameter of the
>>> EnglishGrammaticalStructure constructor, to indicate multi-threaded
>>> behavior, fixes the problem.
>>>>>> Many thanks.
>>>>>> Best regards,
>>> Ioan
>>>>>>>>> On Thursday, December 22, 2011, John Bauer <horatio at gmail.com> wrote:
>>> > Thanks for pointing this out.
>>> >
>>> > If you look at edu.stanford.nlp.trees.EnglishGrammaticalStructure,
>>> > lines 63 and 74, and change "false" to "true", it will modify internal
>>> > lists in a threadsafe manner. Does that make the grammatical
>>> > structures thread safe as well?
>>> >
>>> > John
>>> >
>>> > On Wed, Dec 21, 2011 at 2:14 AM, Ioan Barbulescu <ibarbulescu at gmail.com>
>>> > wrote:
>>> >> Dear parser team
>>> >>
>>> >> I've taken the brand new version of the parser (2.0) for a trip -
>>> >> specifically to test the multi-threading behavior.
>>> >>
>>> >> I must say the parser works for a charm, over multiple threads.
>>> >>
>>> >> However, there is one component which is still not thread-safe.
>>> >> More precisely is the GrammaticalStructureFactory /
>>> >> GrammaticalStructure
>>> >> pair.
>>> >> When I try to run the following snippet of code, multi-threaded, I get
>>> >> an
>>> >> exception (please see the trace below).
>>> >>
>>> >> The reasons why I believe the problem is thread-related are the
>>> >> following:
>>> >> - the error appears only during multi-threaded usage, not in single
>>> >> thread
>>> >> - the error appears consistently (in all runs), but not in a
>>> >> deterministic
>>> >> fashion (sometimes sooner, sometimes later)
>>> >> - the error dis-appear when I synchronize that block of code (still
>>> >> running
>>> >> multi-threaded)
>>> >> - the root error is a ConcurrentModificationException
>>> >>
>>> >> Please take this into consideration, for fixing in the next version.
>>> >>
>>> >> Many thanks :)
>>> >>
>>>>