HTTPServerLoadTest can get stuck about languagetool HOT 8 CLOSED

I guess this could be fixed by using this code in UnifierConfiguration?

equivalenceTypes = Collections.synchronizedMap(new HashMap<EquivalenceTypeLocator, Element>());
equivalenceFeatures = Collections.synchronizedMap(new HashMap<String, List<String>>());

from languagetool.

Hm, seems sensible to me. Did you try it?

from languagetool.

I cannot reproduce the bug, so there's not much I can try (except running the tests and that works).

from languagetool.

Urg, that sure is a problem. The UnifierConfiguration was supposed to be
immutable once the language is completely initialized while the Unifier
(created by a UnifierConfiguration) is stateful. A synchronized map (or
java.util.ConcurrentHashMap) is a temporary workaround, but I'd prefer them
to be cached somehow to improve the speed of creating a JLanguageTool.

I think we should use a ConcurrentHashMap for now, because at least
Unifier.startNextToken uses the keySet() of
UnifierConfiguration.equivalenceFeatures and a Collections.synchronizedMap
is supposed to be synchronized externally in such cases:
--- snip ---
It is imperative that the user manually synchronize on the returned map
when iterating over any of its collection views:
[...]
--- snip ---

btw, I guess it's a good time to share my thoughts on how I wanted to
improve rule loading anyway. The idea is to move loaded rules to either an
own structure or to the language. In both cases the rules are then provided
to the JLanguageTool rather than being loaded by the JLanguageTool itself.
This change allows the (API-)user to cache/hold the rules and set up a
JLanguageTool very fast (in case he uses rules). But I don't know when I
actually start working on it (maybe in the next days, because it's a long
weekend), so I'm fine with implementing a workaround for now, as mentioned
above.

On Tue, Oct 1, 2013 at 8:55 PM, Daniel Naber [email protected]:

I guess this could be fixed by using this code in UnifierConfiguration?

equivalenceTypes = Collections.synchronizedMap(new HashMap<EquivalenceTypeLocator, Element>());
equivalenceFeatures = Collections.synchronizedMap(new HashMap<String, List>());

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/13#issuecomment-25477608
.

from languagetool.

I just committed a fix that seems to work. Well, I could reproduce the problem a few times with the FrenchConcurrencyTest. The HashMaps has been replaced by ConcurrentHashMaps and the ArrayList stored as value in the equivalenceFeatures has been replaced by a CopyOnWriteArrayList. The later change is 'just in case'.

I was not able to reproduce the bug in the HTTPServerLoadTest, so I'm keen to see if you can still reproduce the bug.

from languagetool.

I could never reproduce it, it only happened once (but I didn't try running the test for hours...). Could you please document your fix in CHANGES.txt?

from languagetool.

Done.

from languagetool.

I'll close this issue as it should've been fixed now. Issue #14 might change something on this fix, hopefully the removal of the concurrent collections that were implemented as workaround.

from languagetool.