You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-user@lucene.apache.org by Trejkaz <tr...@trypticon.org> on 2012/02/01 01:16:25 UTC

Lucene appears to use memory maps after unmapping them

Hi all.

I've found a rather frustrating issue which I can't seem to get to the
bottom of.

Our application will crash with an access violation around the time
when the index is closed, with various indications of what's on the
stack, but the common things being SegmentTermEnum.next and
MMapIndexInput.read*.

When it happens, Java doesn't hand us the full stack trace so it's
impossible to track it back to which of our own pieces of code is
calling it in this situation.

I tried adding code to MMapIndexInput to check if it has been closed
at the start of each read method, but that doesn't seem to prevent the
issue either. Obviously there is some kind of situation where two
threads are somehow hitting it at the same time so it gets closed
while some other thread is in the middle of a method call.

I tried changing the code in MMapIndexInput.close to set the array
element to null before cleaning the mapping, but if it's a
multi-threading kind of issue this probably won't help anyway as array
references aren't updated in a thread-safe way (though I guess there
is scope to experiment with the concurrency classes here.)

I tried turning off the unmapping of the buffers using
setUseUnmap(true) - this does prevent the crash, but now the index
files are kept open essentially forever, which prevents other things
from working.

For now I have just disabled memory mapping entirely by creating a
SimpleFSDirectory directly instead of going via the factory method. I
guess this is what the default was originally anyway, so at worst it
will just mean getting old performance figures back again..?

Annoyingly I can't get it to happen in a self-contained unit test,
otherwise I would be submitting a proper bug report. :(

TX

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


RE: Lucene appears to use memory maps after unmapping them

Posted by Uwe Schindler <uw...@thetaphi.de>.
Hi one addition:
In the coming Lucene 3.6 there are more safety checks in MMapDirectory so the SIGSEGV is more unlikely (it tracks cloned index input in a thread safe list on close). But this only *helps* to find the issue, but does not guarantee that your JVM crashes, sorry.

As Robert and Mike noted, check that you app don’t close IndexReaders that are still in use by other threads. SearcherManager is a good idea. Alternatively use refcounting:
- Open IndexReader (this increments its ref count automatically to 1)
- every thread that uses the reader does IR.incRef() and when done does IR.decRef(), ideally using try...finally
- when the main thread wants to recycle the reader it can simply do this by also calling IR.decRef()

This way only when all incRefs have corresponding decRef (the open itself is also an incRef internally) the reader is really closed (it should at the end have IR.getRefCount()==0).

Uwe

-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de


> -----Original Message-----
> From: Trejkaz [mailto:trejkaz@trypticon.org]
> Sent: Wednesday, February 01, 2012 2:33 AM
> To: java-user@lucene.apache.org
> Subject: Re: Lucene appears to use memory maps after unmapping them
> 
> On Wed, Feb 1, 2012 at 11:30 AM, Robert Muir <rc...@gmail.com> wrote:
> > the problem is caused by searching indexreaders after you closed them.
> >
> > in general we can try to add more and more safety, but at the end of
> > the day, if you close an indexreader while a search is running, you will have
> problems.
> >
> > So be careful to only close indexreaders that are no longer in use!
> >
> > For multithreaded code you might want to investigate IndexReader's
> > reference counting mechanism or SearcherManager to help you track
> > this.
> 
> So rather than flagging closed = true when close() is called and hitting someone
> who tries to read data, essentially try and do the
> reverse- if a search is still running and close() is called, throw an exception to
> them?
> 
> Some of our underlying code isn't only doing searches... so it still might not
> catch all usages. But it should catch the more easily found ones I hope.
> 
> I guess I would need to create a stack trace every time someone starts a new
> search for this to work, so that there is some point of blame when the
> exception fires. Otherwise I would be back at step 1, having no idea who did the
> wrong thing.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Lucene appears to use memory maps after unmapping them

Posted by Michael McCandless <lu...@mikemccandless.com>.
On Tue, Jan 31, 2012 at 9:42 PM, Trejkaz <tr...@trypticon.org> wrote:

> So when we close() our own TextIndex wrapper class, it would call
> decRef() - but if another thread is still using the index, this call
> to decRef() wouldn't actually close the reader. IMO, this wouldn't
> really satisfy the meaning of "close" for the TextIndex.
>
> Rather, I would prefer to track who is currently using it (which is
> similar to reference counting) and throw some exception which points
> the finger at some code which hasn't correctly terminated so that it
> can be fixed.
>
> After all, we don't close the text index until after all the GUI
> components have been closed. Each of those is supposed to clean itself
> up, which includes cancelling any tasks which might be accessing the
> text index. If any searches are still going on by the time it gets to
> close(), there is a bug somewhere.

I think you should separate safety (don't close a reader if another
thread is still using it) from bugs in the app.

First and foremost you need to be safe...

Secondarily, figure out why threads are still using the reader when
you don't expect it...

> P.S. Why is the SearcherManager API written in such a way that it
> makes it easy to fail to return the searcher?  Wouldn't it make more
> sense to design it like this?
>
>    manager.execute(new IndexSearcherLogic() {
>        public void withSearcher(IndexSearcher searcher) throws IOException {
>            // .. logic which might well throw an exception
>        }
>    });
>
> This way if their block throws an exception you're not relying on them
> remembering to release it in a finally block and it makes it nearly
> impossible to do the wrong thing.

While I do agree this would make things "safer", I don't like APIs
that "invert"; I think it makes the API harder to consume...

Also, if the app really wants this API, they can build it out on top
of the non-inverted API that Searcher/NRTManager provide today...

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Lucene appears to use memory maps after unmapping them

Posted by Trejkaz <tr...@trypticon.org>.
On Wed, Feb 1, 2012 at 1:14 PM, Robert Muir <rc...@gmail.com> wrote:
>
> No, I don't think you should use close at all, because your problem is
> you are calling close() when its unsafe to do so (you still have other
> threads that try to search the reader after you closed it).
>
> Instead of trying to fix the bugs in your code, I suggested using
> SearcherManager, which uses IndexReaders reference counting API (or
> you can use that directly alternatively).

So when we close() our own TextIndex wrapper class, it would call
decRef() - but if another thread is still using the index, this call
to decRef() wouldn't actually close the reader. IMO, this wouldn't
really satisfy the meaning of "close" for the TextIndex.

Rather, I would prefer to track who is currently using it (which is
similar to reference counting) and throw some exception which points
the finger at some code which hasn't correctly terminated so that it
can be fixed.

After all, we don't close the text index until after all the GUI
components have been closed. Each of those is supposed to clean itself
up, which includes cancelling any tasks which might be accessing the
text index. If any searches are still going on by the time it gets to
close(), there is a bug somewhere.

TX


P.S. Why is the SearcherManager API written in such a way that it
makes it easy to fail to return the searcher?  Wouldn't it make more
sense to design it like this?

    manager.execute(new IndexSearcherLogic() {
        public void withSearcher(IndexSearcher searcher) throws IOException {
            // .. logic which might well throw an exception
        }
    });

This way if their block throws an exception you're not relying on them
remembering to release it in a finally block and it makes it nearly
impossible to do the wrong thing.

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Lucene appears to use memory maps after unmapping them

Posted by Robert Muir <rc...@gmail.com>.
On Tue, Jan 31, 2012 at 8:32 PM, Trejkaz <tr...@trypticon.org> wrote:
> On Wed, Feb 1, 2012 at 11:30 AM, Robert Muir <rc...@gmail.com> wrote:
>> the problem is caused by searching indexreaders after you closed them.
>>
>> in general we can try to add more and more safety, but at the end of the day,
>> if you close an indexreader while a search is running, you will have problems.
>>
>> So be careful to only close indexreaders that are no longer in use!
>>
>> For multithreaded code you might want to investigate IndexReader's
>> reference counting mechanism or SearcherManager to help you track
>> this.
>
> So rather than flagging closed = true when close() is called and
> hitting someone who tries to read data, essentially try and do the
> reverse- if a search is still running and close() is called, throw an
> exception to them?
>

No, I don't think you should use close at all, because your problem is
you are calling close() when its unsafe to do so (you still have other
threads that try to search the reader after you closed it).

Instead of trying to fix the bugs in your code, I suggested using
SearcherManager, which uses IndexReaders reference counting API (or
you can use that directly alternatively).

-- 
lucidimagination.com

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Lucene appears to use memory maps after unmapping them

Posted by Trejkaz <tr...@trypticon.org>.
On Wed, Feb 1, 2012 at 11:30 AM, Robert Muir <rc...@gmail.com> wrote:
> the problem is caused by searching indexreaders after you closed them.
>
> in general we can try to add more and more safety, but at the end of the day,
> if you close an indexreader while a search is running, you will have problems.
>
> So be careful to only close indexreaders that are no longer in use!
>
> For multithreaded code you might want to investigate IndexReader's
> reference counting mechanism or SearcherManager to help you track
> this.

So rather than flagging closed = true when close() is called and
hitting someone who tries to read data, essentially try and do the
reverse- if a search is still running and close() is called, throw an
exception to them?

Some of our underlying code isn't only doing searches... so it still
might not catch all usages. But it should catch the more easily found
ones I hope.

I guess I would need to create a stack trace every time someone starts
a new search for this to work, so that there is some point of blame
when the exception fires. Otherwise I would be back at step 1, having
no idea who did the wrong thing.

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Re: Lucene appears to use memory maps after unmapping them

Posted by Robert Muir <rc...@gmail.com>.
the problem is caused by searching indexreaders after you closed them.

in general we can try to add more and more safety, but at the end of the day,
if you close an indexreader while a search is running, you will have problems.

So be careful to only close indexreaders that are no longer in use!

For multithreaded code you might want to investigate IndexReader's
reference counting mechanism or SearcherManager to help you track
this.

On Tue, Jan 31, 2012 at 7:16 PM, Trejkaz <tr...@trypticon.org> wrote:
> Hi all.
>
> I've found a rather frustrating issue which I can't seem to get to the
> bottom of.
>
> Our application will crash with an access violation around the time
> when the index is closed, with various indications of what's on the
> stack, but the common things being SegmentTermEnum.next and
> MMapIndexInput.read*.
>
> When it happens, Java doesn't hand us the full stack trace so it's
> impossible to track it back to which of our own pieces of code is
> calling it in this situation.
>
> I tried adding code to MMapIndexInput to check if it has been closed
> at the start of each read method, but that doesn't seem to prevent the
> issue either. Obviously there is some kind of situation where two
> threads are somehow hitting it at the same time so it gets closed
> while some other thread is in the middle of a method call.
>
> I tried changing the code in MMapIndexInput.close to set the array
> element to null before cleaning the mapping, but if it's a
> multi-threading kind of issue this probably won't help anyway as array
> references aren't updated in a thread-safe way (though I guess there
> is scope to experiment with the concurrency classes here.)
>
> I tried turning off the unmapping of the buffers using
> setUseUnmap(true) - this does prevent the crash, but now the index
> files are kept open essentially forever, which prevents other things
> from working.
>
> For now I have just disabled memory mapping entirely by creating a
> SimpleFSDirectory directly instead of going via the factory method. I
> guess this is what the default was originally anyway, so at worst it
> will just mean getting old performance figures back again..?
>
> Annoyingly I can't get it to happen in a self-contained unit test,
> otherwise I would be submitting a proper bug report. :(
>
> TX
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org
>



-- 
lucidimagination.com

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org