fail tests on threadlocal leaks [LUCENE-6335]

[asfimport]

I know this requires us to do things like close our Analyzers in tests, but I think we should do it. just experimenting i found other leaks, e.g. in LineFileDocs.

   [junit4] ERROR   0.00s J0 | TestForceMergeForever (suite) <<<
   [junit4]    > Throwable #1: java.lang.IllegalStateException: ThreadLocal leaks were found: 
   [junit4]    > 1. thread=SUITE-TestForceMergeForever-seed#[AF7141C55A57350E]-worker value=WeakReference<HashMap<?,Analyzer$TokenStreamComponents>>
   [junit4]    > 2. thread=SUITE-TestForceMergeForever-seed#[AF7141C55A57350E]-worker value=LineFileDocs$DocState

---

Migrated from LUCENE-6335 by Robert Muir (@rmuir), updated Mar 05 2015 Attachments: LUCENE-6335.patch (versions: 3)

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

here is a REALLY hacky patch. I based it on tomcat's threadlocal leak detector, but with some simpler/imperfect logic as a start, since e.g. we aren't wrapping tests in custom classloader or any of that stuff.

I only fixed TestDemo and LineFileDocs so far and didnt look much more at failures.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

A lot of the hacky stuff was to get something useful enough to track down the leak. It makes the code ugly to print "WeakReference<HashMap<?,Analyzer$TokenStreamComponents>>" but this is much more useful than just "java.lang.ref.WeakReference"

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

updated patch: I started fixing some of the analyzers tests.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Updated patch. I fixed a few tests. There are still lots of leaks and systematic problems.

Only one real bugfix, SynonymFilterFactory leaked an analyzer it created internally. Otherwise it is just test fixes... I plan to commit these test fixes soon and just update patch with the test-framework stuff.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

I reviewed the test rule. It looks crazily bound to the default JDK implementation (all that reflection) but there seems to be no other way to get hold of the thread locals map, so be it.

Overall it looks good. I'm slightly concerned with this:

+    ClassLoader cl = clazz.getClassLoader();
+    // our crazy heuristic: threadlocal contains something from non-bootstrap loader.
+    if (cl != null) {
+      throw new ThreadLocalLeakException(getPrettyClassName(clazz));
+    }

because I planned to isolate the test runner's classes from the test classes at some point by forking a separate classloader... But there are workarounds for this and these can be applied and explored when the actual problem shows up.

+1

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

I plan to commit these test fixes soon and just update patch with the test-framework stuff.

+1

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

because I planned to isolate the test runner's classes from the test classes at some point by forking a separate classloader... But there are workarounds for this and these can be applied and explored when the actual problem shows up.

Tomcat code is actually doing all this in a custom classloader.

But yeah, there are thousands more tests (and some real bugs) to fix before we can add the test rule. I have concerns about this craziness too, but it does seem to work at least for our purposes in lucene.

[asfimport]

ASF subversion and git services (migrated from JIRA)

Commit 1664404 from @rmuir in branch 'dev/trunk' https://svn.apache.org/r1664404

LUCENE-6335: test fixes, and one real fix to synonymfilterfactory (missing analyzer.close)

[asfimport]

ASF subversion and git services (migrated from JIRA)

Commit 1664421 from @rmuir in branch 'dev/branches/branch_5x' https://svn.apache.org/r1664421

LUCENE-6335: test fixes, and one real fix to synonymfilterfactory (missing analyzer.close)