Actually I did change all those code to the double-locking technique because the IStemmer is not thread-safe. And I was pretty sure that there's no problem with the double-locking technique. Damn it! Well, now I know better. Thanks for hint.
The idea behind using the double-locking was just to reduce I/O and memory consumption on the first usage of languages, because they happen to be "hardcore" due to all that file loading and whatever. So I simply blocked other threads from doing the exactly same work and let them just wait (synchronize) until the stuff is initialized. I hoped to spend more time on the project to implement a better solution as soon as my knowledge of the code base grows, but I rarely had free time in the last months :( But I'm glad that you take a look into it. Thanks! On Thu, Feb 20, 2014 at 2:06 PM, Marcin Miłkowski <list-addr...@wp.pl> wrote: > W dniu 2014-02-16 22:05, Dominique Pellé pisze: >> Daniel Naber <daniel.na...@languagetool.org >> <mailto:daniel.na...@languagetool.org>> wrote: >> >> > On 2014-02-01 17:08, Xavi Ivars wrote >> > >> >>> wget -q -T 1 localhost:8081/Languages >> >> >> >> If it works, then I do nothing, but if it doesn't succeed, then I kill >> >> the process (if it's still there) and I restart it again. >> > >> > That wasn't enough for me, as sometimes the server had a high load but >> > still replied to some requests or so, so I'm using a script that checks >> > the load the process causes. It's a bad hack, we shouldn't stop fixing >> > the actual reason for these problems (I sent you the script anyway via >> > email)... >> >> Hi >> >> Regarding the server hanging problem (which I believe is still >> happening, but I'd love to be wrong), it's likely the be a >> race condition. One thing I noticed in the source code, is the >> frequent use of "double-checked locking" pattern (or anti-pattern). >> >> Here is an example taken from >> languagetool/languagetool-language-modules/de/src/main/java/org/languagetool/language/German.java: >> >> 99 @Override >> 100 public Tagger getTagger() { >> 101 if (tagger == null) { >> 102 synchronized (this) { >> 103 if (tagger == null) { >> 104 tagger = new GermanTagger(); >> 105 } >> 106 } >> 107 } >> 108 return tagger; >> 109 } >> >> This optimization is not safe as far as I know. In fact, >> I just looked it up and wikipedia confirms it. See: >> >> http://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java >> >> == BEGIN QUOTE from wikipedia == >> [...] this technique has many subtle problems and should >> usually be avoided. >> >> [...] >> >> One of the dangers of using double-checked locking in J2SE 1.4 >> (and earlier versions) is that it will often appear to work: it is not >> easy to distinguish between a correct implementation of the >> technique and one that has subtle problems. Depending on >> the compiler, the interleaving of threads by the scheduler and >> the nature of other concurrent system activity, failures resulting >> from an incorrect implementation of double-checked locking >> may only occur intermittently. Reproducing the failures can >> be difficult. >> == END QUOTE == >> >> Besides the example in... >> languagetool/languagetool-language-modules/de/src/main/java/org/languagetool/language/German.java >> >> ... I see the double-checked lock anti-pattern being used >> at least here: >> >> languagetool-language-modules/ca/src/main/java/org/languagetool/tagging/ca/CatalanTagger.java >> >> languagetool-language-modules/pl/src/main/java/org/languagetool/synthesis/pl/PolishSynthesizer.java >> >> languagetool-language-modules/de/src/main/java/org/languagetool/tagging/de/GermanTagger.java >> (this is looks worse, as the check involves multiple variables) >> >> languagetool-core/src/main/java/org/languagetool/synthesis/BaseSynthesizer.java >> >> languagetool-core/src/main/java/org/languagetool/tagging/BaseTagger.java >> >> The wikipedia page proposes Java alternatives. See: >> http://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java >> >> Of course, it may not be the root cause of server instabilities, >> but it looks worth fixing it. > > I fixed some of those but I also ran a stress test using JMeter. It > turns out that we had a major problem in BaseSynthesiser.lookup -- > IStemmer.lookup() is not thread-safe, so we need to acquire a look > before calling it. After adding a lock, lots of exceptions are gone. > This may explain a lot of crashes. > > I'll look into other cases as well. > > Regards, > Marcin > > ------------------------------------------------------------------------------ > Managing the Performance of Cloud-Based Applications > Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. > Read the Whitepaper. > http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk > _______________________________________________ > Languagetool-devel mailing list > Languagetool-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/languagetool-devel ------------------------------------------------------------------------------ Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk _______________________________________________ Languagetool-devel mailing list Languagetool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/languagetool-devel
