lucene-lucene-net-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ayende Rahien <aye...@ayende.com>
Subject Re: [Lucene.Net] Memory leaks
Date Wed, 24 Aug 2011 13:26:00 GMT
That make sense, and explains how it works.
Basically, there is no rooted hard reference anywhere to the values. It
looks like the actual problem is happening when you have instances in a
thread that is no longer used, the _values_ aren't weak references, and they
are leaking.


On Wed, Aug 24, 2011 at 3:41 PM, Itamar Syn-Hershko <itamar@code972.com>wrote:

> I moved this to a private conversation by accident, so back on the mailing
> list now.
>
> There seem to be a fundamental difference between the original Java impl
> and Lucene.NET's, which may as well be causing the leak.
>
> In the Java version ThreadLocal is a weak reference, and a hard reference
> is persisted as well along with the thread holding it (using a synchronized
> map). This allows to easily identify stale refs - if the thread holding the
> weak ref is not alive anymore, the hard ref is removed and the GC can
> process it.
>
> In Lucene.NET's implementation only a static map of weak references is
> kept. If I read this right, unlike Java's implementation where hard
> references are kept and dereferenced  from within the ThreadLocal when the
> thread is dead, here there is nothing that will tell the GC to reclaim the
> memory referenced by the Weak ref. The Weak-Hard combination that is missing
> here is probably whats causing all this mess.
>
> Ayende's solution which uses explicit disposal is virtually the missing
> piece but in different clothing, since it requires changes to the core. If
> the .NET version was made to match Java's that probably won't be necessary.
>
> Before I rewrite CloseableThreadLocal to match the Java implementation - is
> there a reason it was made this way?
>
>
> On Wed, Aug 24, 2011 at 10:51 AM, Ayende Rahien <ayende@ayende.com> wrote:
>
>> Itamar,
>> Can you try writing the same code in Java and see if we get the same
>> results?
>>
>>
>> On Tue, Aug 23, 2011 at 8:58 PM, Itamar Syn-Hershko <itamar@code972.com>wrote:
>>
>>> How come the Java version doesn't suffer from this issue? if your answer
>>> is going to be due to differences with the JVM, then IDisposable may well be
>>> our best option here
>>>
>>>
>>> On Tue, Aug 23, 2011 at 8:51 PM, Digy <digydigy@gmail.com> wrote:
>>>
>>>> Exactly, but I couldn’t find a nice solution yet.  Any help welcomed J*
>>>> ***
>>>>
>>>> ** **
>>>>
>>>> Btw, Can you open a new issue for this ? ****
>>>>
>>>> Other  people may also want to work on it.****
>>>>
>>>> ** **
>>>>
>>>> https://issues.apache.org/jira/browse/LUCENENET****
>>>>
>>>> ** **
>>>>
>>>> Thanks,****
>>>>
>>>> ** **
>>>>
>>>> DIGY****
>>>>
>>>> ** **
>>>>
>>>> *From:* Ayende Rahien [mailto:ayende@ayende.com]
>>>> *Sent:* Tuesday, August 23, 2011 8:32 PM
>>>>
>>>> *To:* Digy
>>>> *Cc:* Itamar Syn-Hershko
>>>> *Subject:* Re: [Lucene.Net] Memory leaks****
>>>>
>>>> ** **
>>>>
>>>> I think that the main issue is that you have per thread state, but no
>>>> way of releasing this per thread state when you are closing the searcher.
>>>> ****
>>>>
>>>> On Tue, Aug 23, 2011 at 7:59 PM, Digy <digydigy@gmail.com> wrote:****
>>>>
>>>> Yes, IndexSearcher is expected to be thread safe. ****
>>>>
>>>> I am not saying, there is a misuse of it.  Instead I try to understand
>>>> the real cause of it.  IDisposable solution seems to hide the real bug.
>>>> ****
>>>>
>>>>  ****
>>>>
>>>> DIGY****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>> *From:* Ayende Rahien [mailto:ayende@ayende.com]
>>>> *Sent:* Tuesday, August 23, 2011 7:48 PM
>>>> *To:* Digy****
>>>>
>>>>
>>>> *Cc:* Itamar Syn-Hershko
>>>> *Subject:* Re: [Lucene.Net] Memory leaks****
>>>>
>>>>  ****
>>>>
>>>> Digy,****
>>>>
>>>> IndexSearcher is thread safe, right?****
>>>>
>>>> From the docs (
>>>> http://lucene.apache.org/java/3_0_1/api/core/org/apache/lucene/search/IndexSearcher.html
>>>> ):****
>>>>
>>>> *NOTE*: IndexSearcher instances are completely thread safe, meaning
>>>> multiple threads can call any of its methods, concurrently. If your
>>>> application requires external synchronization, you should *not* synchronize
>>>> on the IndexSearcher instance; use your own (non-Lucene) objects
>>>> instead.****
>>>>
>>>> The problem is that when you use an index searcher in a different thread
>>>> then the one you close it on, you are going to leak some memory. ****
>>>>
>>>>  ****
>>>>
>>>> On Tue, Aug 23, 2011 at 7:35 PM, Digy <digydigy@gmail.com> wrote:****
>>>>
>>>> If I change the searching code as below, It works as expected****
>>>>
>>>>  ****
>>>>
>>>>             for (int i = 0; i < 150; i++)****
>>>>
>>>>             {****
>>>>
>>>>                 ExecuteInParallel(() =>****
>>>>
>>>>                     {****
>>>>
>>>>                         var searchers = new[]****
>>>>
>>>>                         {****
>>>>
>>>>                             new IndexSearcher(directories[0], true),***
>>>> *
>>>>
>>>>                             new IndexSearcher(directories[1], true),***
>>>> *
>>>>
>>>>                         };****
>>>>
>>>>  ****
>>>>
>>>>                         {****
>>>>
>>>>                             var topDocs = searchers[0].Search(new
>>>> TermQuery(new Term("val", "test")), 15);****
>>>>
>>>>                             for (int j = 0; j < topDocs.totalHits; j++)
>>>> ****
>>>>
>>>>                             {****
>>>>
>>>>                                 searchers[0].Doc(j);****
>>>>
>>>>                             }****
>>>>
>>>>                             topDocs = searchers[1].Search(new
>>>> TermQuery(new Term("val", "test")), 15);****
>>>>
>>>>                             for (int j = 0; j < topDocs.totalHits; j++)
>>>> ****
>>>>
>>>>                             {****
>>>>
>>>>                                 searchers[1].Doc(j);****
>>>>
>>>>                             }****
>>>>
>>>>                             TryAddSlot(slotsField,
>>>> allSlotsFromAllThreads);****
>>>>
>>>>                         };****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>>                         foreach (var indexSearcher in searchers)****
>>>>
>>>>                         {****
>>>>
>>>>                             indexSearcher.Close();****
>>>>
>>>>                         }****
>>>>
>>>>                     });****
>>>>
>>>>             }****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>> Since IndexSearcher is closed in the thread where caching is done.****
>>>>
>>>>  ****
>>>>
>>>> I have to think about it some more but I don’t think that adding
>>>> IDisposables to some classes would be a good solution. It is more related
to
>>>> threading issues.****
>>>>
>>>>  ****
>>>>
>>>> Anyway, I am working on it.****
>>>>
>>>>  ****
>>>>
>>>> DIGY****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>> *From:* Ayende Rahien [mailto:ayende@ayende.com]
>>>> *Sent:* Tuesday, August 23, 2011 6:05 PM
>>>> *To:* digy digy
>>>> *Cc:* Itamar Syn-Hershko
>>>> *Subject:* Re: [Lucene.Net] Memory leaks****
>>>>
>>>>  ****
>>>>
>>>> It appears that we need to use searching as well to reproduce this.****
>>>>
>>>> Here is another version of the program.****
>>>>
>>>> I apologize for the code, but I basically needed to ensure that we would
>>>> get something resembling what we have in RavenDB, which is a multi threaded
>>>> access pattern.****
>>>>
>>>>  ****
>>>>
>>>> On Tue, Aug 23, 2011 at 5:43 PM, digy digy <digydigy@gmail.com> wrote:*
>>>> ***
>>>>
>>>> If I add ****
>>>>
>>>>    GC.Collect(); ****
>>>>
>>>> after ****
>>>>
>>>>    simpleAnalyzer.Close();****
>>>>
>>>> It outputs zeroes.  Am I missing something?****
>>>>
>>>>  ****
>>>>
>>>> DIGY****
>>>>
>>>>  ****
>>>>
>>>> On Tue, Aug 23, 2011 at 5:07 PM, Itamar Syn-Hershko <itamar@code972.com>
>>>> wrote:****
>>>>
>>>> See attached.****
>>>>
>>>>  ****
>>>>
>>>> The key to this problem is multi-threading. In this sample app, some of
>>>> the threads do not dispose of their resources properly, even though Close
is
>>>> called correctly.****
>>>>
>>>>  ****
>>>>
>>>> The suggested patch is confirmed to fix this behavior, but there may be
>>>> other pit-falls our use cases didn't reveal.****
>>>>
>>>>  ****
>>>>
>>>> Itamar.****
>>>>
>>>>  ****
>>>>
>>>> On Tue, Aug 23, 2011 at 4:19 PM, digy digy <digydigy@gmail.com> wrote:*
>>>> ***
>>>>
>>>> Hi Itamar,****
>>>>
>>>>  ****
>>>>
>>>> Can you send a simple self-contained test case showing the bug?****
>>>>
>>>>  ****
>>>>
>>>> DIGY****
>>>>
>>>>  ****
>>>>
>>>> On Tue, Aug 23, 2011 at 2:41 PM, Itamar Syn-Hershko <itamar@code972.com>
>>>> wrote:****
>>>>
>>>> Hi,****
>>>>
>>>>  ****
>>>>
>>>> We recently discovered another memory leak within Lucene.NET, that is
>>>> related to LUCENENET-358 (CloseableThreadLocal memory leak in
>>>> LocalDataStoreSlot). As it turns out, disposables weren't disposed properly.
>>>> With RavenDB, that caused memory consumption to always grow.****
>>>>
>>>>  ****
>>>>
>>>> We ended up modifying some core files to assure proper disposal, and
>>>> things are looking better now. We are still testing, and will follow up if
>>>> more fixes will be required.****
>>>>
>>>>  ****
>>>>
>>>> Here are the relevant diffs in our fork of Lucene.NET:****
>>>>
>>>>  ****
>>>>
>>>>
>>>> https://github.com/ayende/ravendb/commit/a5031049599968f85e733f22a1ed568c09703c30
>>>> ****
>>>>
>>>>
>>>> https://github.com/ayende/ravendb/commit/bd9ca1ab154732d27985c2d237917af8bbd5fbf3
>>>> ****
>>>>
>>>>  ****
>>>>
>>>> The full discussion and bug hunting is here:****
>>>>
>>>>  ****
>>>>
>>>>
>>>> http://groups.google.com/group/ravendb/browse_thread/thread/9c6340987407aece/1d86d6a61175233c
>>>> ****
>>>>
>>>>  ****
>>>>
>>>> I'm also attaching a diff between our fork to the Lucene.Net_2_9_2_RC2
>>>> tag. Will be happy to see this incorporated into 2.9.4 (and for it to be
>>>> released already...).****
>>>>
>>>>  ****
>>>>
>>>> Itamar.****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>>  ****
>>>>
>>>> ** **
>>>>
>>>
>>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message