Mailing List Archive

ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause
I was messing around with something that was resulting in
AlreadyClosedException being thrown and I noticed that we weren't
tracking the exception that caused it. I found this in
ByteBufferIndexInput:

// the unused parameter is just to silence javac about unused variables
AlreadyClosedException alreadyClosed(RuntimeException unused) {
- return new AlreadyClosedException("Already closed: " + this);
+ return new AlreadyClosedException("Already closed: " + this, unused);
}

and added the cause there, which helped me find and fix my wicked
ways. Is there a reason we decided not to wrap the "unused"
RuntimeException there?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
FWIW: The choice to ignore the original exception goes back to here...

https://issues.apache.org/jira/browse/LUCENE-3588

...circa 2011, where it was focused on catching NPE and throwing
AlreadyClosedException instead, w/o any particular discussion as to why to
throw away the original NPE.

If i had to guess it's simply because at that time AlreadyClosedException
didn't support wrapping any other Throwable. That wasn't added until
LUCENE-5958 (circa 2014) which was focused on making sure "tragic" errors
kept a record of what caused the tragedy, and then include that as the
'cause' of the AlreadyClosedExceptions throw by 'ensureOpen()'

There didn't seem to be any discussion at that time about reviewing other
code that might be throwing AlreadyClosedException from a 'catch' block
that could also be updated to include the cause.

I'd say open a PR to review & update all code that results in
AlreadyClosedException originating from a catch block?



: Date: Tue, 17 Oct 2023 11:24:03 -0400
: From: Michael Sokolov <msokolov@gmail.com>
: Reply-To: dev@lucene.apache.org
: To: Lucene Dev <dev@lucene.apache.org>
: Subject: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't
: track its cause
:
: I was messing around with something that was resulting in
: AlreadyClosedException being thrown and I noticed that we weren't
: tracking the exception that caused it. I found this in
: ByteBufferIndexInput:
:
: // the unused parameter is just to silence javac about unused variables
: AlreadyClosedException alreadyClosed(RuntimeException unused) {
: - return new AlreadyClosedException("Already closed: " + this);
: + return new AlreadyClosedException("Already closed: " + this, unused);
: }
:
: and added the cause there, which helped me find and fix my wicked
: ways. Is there a reason we decided not to wrap the "unused"
: RuntimeException there?
:
: ---------------------------------------------------------------------
: To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
: For additional commands, e-mail: dev-help@lucene.apache.org
:
:

-Hoss
http://www.lucidworks.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Hi,

please don't add the NPE here as cause (except for debugging). The NPE
is only catched to NOT add extra checks in the highly performance
sensitive code. Actually the NPE is catched to detect the case where the
bytebuffer was already unset to trigger the already closed. The code
uses setting the buffers to NULL to signal cause, but it does NOT add a
NULL check everywhere. This allows Hotspot to compile this code without
any bounds checks and signal the AlreadyClosedException only when a NPE
happens. Adding the NPE as cause would bring confusion to end user, as
we only want to tell that IndexInput was closed, but the NPE should be
kept behind scenes as it would be a support nightmare ("your code has no
good null checks, it is broken"). The NPE is a signal here, not the cause.

I think the issue you have seen may be cause by passing a NULL parameter
to one of the methods like a float array to readFloats(). This is not
detected (P.S.: this also affects MemorySegmentIndexInput).

I can looks at the code to figure out how to better detect the NPE when
parameters of methods are NULL, but no way to add the cause here. I
would say: If you have to debug, do it temporarily or ose another
dircetory impl.

Uwe

Am 20.10.2023 um 21:20 schrieb Chris Hostetter:
> FWIW: The choice to ignore the original exception goes back to here...
>
> https://issues.apache.org/jira/browse/LUCENE-3588
>
> ...circa 2011, where it was focused on catching NPE and throwing
> AlreadyClosedException instead, w/o any particular discussion as to why to
> throw away the original NPE.
>
> If i had to guess it's simply because at that time AlreadyClosedException
> didn't support wrapping any other Throwable. That wasn't added until
> LUCENE-5958 (circa 2014) which was focused on making sure "tragic" errors
> kept a record of what caused the tragedy, and then include that as the
> 'cause' of the AlreadyClosedExceptions throw by 'ensureOpen()'
>
> There didn't seem to be any discussion at that time about reviewing other
> code that might be throwing AlreadyClosedException from a 'catch' block
> that could also be updated to include the cause.
>
> I'd say open a PR to review & update all code that results in
> AlreadyClosedException originating from a catch block?
>
>
>
> : Date: Tue, 17 Oct 2023 11:24:03 -0400
> : From: Michael Sokolov <msokolov@gmail.com>
> : Reply-To: dev@lucene.apache.org
> : To: Lucene Dev <dev@lucene.apache.org>
> : Subject: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't
> : track its cause
> :
> : I was messing around with something that was resulting in
> : AlreadyClosedException being thrown and I noticed that we weren't
> : tracking the exception that caused it. I found this in
> : ByteBufferIndexInput:
> :
> : // the unused parameter is just to silence javac about unused variables
> : AlreadyClosedException alreadyClosed(RuntimeException unused) {
> : - return new AlreadyClosedException("Already closed: " + this);
> : + return new AlreadyClosedException("Already closed: " + this, unused);
> : }
> :
> : and added the cause there, which helped me find and fix my wicked
> : ways. Is there a reason we decided not to wrap the "unused"
> : RuntimeException there?
> :
> : ---------------------------------------------------------------------
> : To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> : For additional commands, e-mail: dev-help@lucene.apache.org
> :
> :
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail: uwe@thetaphi.de


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Hi Hoss,

Hi, see my other response, this is not the reason why it isn't wrapped:

> If i had to guess it's simply because at that time AlreadyClosedException
> didn't support wrapping any other Throwable. That wasn't added until
> LUCENE-5958 (circa 2014) which was focused on making sure "tragic" errors
> kept a record of what caused the tragedy, and then include that as the
> 'cause' of the AlreadyClosedExceptions throw by 'ensureOpen()'

See my other mail for an explanation, in short: The NPE is only used as
signal and not as error condition and MUST be hidden from end user
(except for debbugging).

Uwe

--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail:uwe@thetaphi.de
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Hi,

I have a good idea: We should only wrap as AlreadyClosedException if and
only if the bytebuffers/memorysegemnts are null (see . In all other
cases rethrow the NPE:

  AlreadyClosedException alreadyClosed(RuntimeException npe) {

    if (npe == null || this.buffers == null) { // buffers == null if input closed!
return new AlreadyClosedException("Already closed: " + this);
}
throw npe;

  }

(this would need the same change in all MemorySegmentIndexInput in a
similar way).

This would keep the NPE on wrong usage, but in the case of a closed
ByteBufferIndexInput / MemorySegmentIndexInput it would throw plain
AlreadyClosedEx.

I can provide a PR, but give me a week, I am very busy.

Uwe

Am 21.10.2023 um 10:01 schrieb Uwe Schindler:
> Hi,
>
> please don't add the NPE here as cause (except for debugging). The NPE
> is only catched to NOT add extra checks in the highly performance
> sensitive code. Actually the NPE is catched to detect the case where
> the bytebuffer was already unset to trigger the already closed. The
> code uses setting the buffers to NULL to signal cause, but it does NOT
> add a NULL check everywhere. This allows Hotspot to compile this code
> without any bounds checks and signal the AlreadyClosedException only
> when a NPE happens. Adding the NPE as cause would bring confusion to
> end user, as we only want to tell that IndexInput was closed, but the
> NPE should be kept behind scenes as it would be a support nightmare
> ("your code has no good null checks, it is broken"). The NPE is a
> signal here, not the cause.
>
> I think the issue you have seen may be cause by passing a NULL
> parameter to one of the methods like a float array to readFloats().
> This is not detected (P.S.: this also affects MemorySegmentIndexInput).
>
> I can looks at the code to figure out how to better detect the NPE
> when parameters of methods are NULL, but no way to add the cause here.
> I would say: If you have to debug, do it temporarily or ose another
> dircetory impl.
>
> Uwe
>
> Am 20.10.2023 um 21:20 schrieb Chris Hostetter:
>> FWIW: The choice to ignore the original exception goes back to here...
>>
>> https://issues.apache.org/jira/browse/LUCENE-3588
>>
>> ...circa 2011, where it was focused on catching NPE and throwing
>> AlreadyClosedException instead, w/o any particular discussion as to
>> why to
>> throw away the original NPE.
>>
>> If i had to guess it's simply because at that time
>> AlreadyClosedException
>> didn't support wrapping any other Throwable.  That wasn't added until
>> LUCENE-5958 (circa 2014) which was focused on making sure "tragic"
>> errors
>> kept a record of what caused the tragedy, and then include that as the
>> 'cause' of the AlreadyClosedExceptions throw by 'ensureOpen()'
>>
>> There didn't seem to be any discussion at that time about reviewing
>> other
>> code that might be throwing AlreadyClosedException from a 'catch' block
>> that could also be updated to include the cause.
>>
>> I'd say open a PR to review & update all code that results in
>> AlreadyClosedException originating from a catch block?
>>
>>
>>
>> : Date: Tue, 17 Oct 2023 11:24:03 -0400
>> : From: Michael Sokolov <msokolov@gmail.com>
>> : Reply-To: dev@lucene.apache.org
>> : To: Lucene Dev <dev@lucene.apache.org>
>> : Subject: ByteBufferIndexInput.alreadyClosed creates an exception
>> that doesn't
>> :     track its cause
>> :
>> : I was messing around with something that was resulting in
>> : AlreadyClosedException being thrown and I noticed that we weren't
>> : tracking the exception that caused it. I found this in
>> : ByteBufferIndexInput:
>> :
>> :    // the unused parameter is just to silence javac about unused
>> variables
>> :    AlreadyClosedException alreadyClosed(RuntimeException unused) {
>> : -    return new AlreadyClosedException("Already closed: " + this);
>> : +    return new AlreadyClosedException("Already closed: " + this,
>> unused);
>> :    }
>> :
>> : and added the cause there, which helped me find and fix my wicked
>> : ways. Is there a reason we decided not to wrap the "unused"
>> : RuntimeException there?
>> :
>> : ---------------------------------------------------------------------
>> : To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> : For additional commands, e-mail: dev-help@lucene.apache.org
>> :
>> :
>>
>> -Hoss
>> http://www.lucidworks.com/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail:uwe@thetaphi.de
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Hi Mike,

here is the PR improving the situation while not making the signalling
NPE visible to code: https://github.com/apache/lucene/pull/12705

In MemorySegmentIndexInput theres also InvalidStateException which
cannot be throws on wrong parameters. This one is also internal and
always mapped to AlreadyClosedException (also without cause).

Uwe

Am 21.10.2023 um 10:23 schrieb Uwe Schindler:
>
> Hi,
>
> I have a good idea: We should only wrap as AlreadyClosedException if
> and only if the bytebuffers/memorysegemnts are null (see . In all
> other cases rethrow the NPE:
>
>   AlreadyClosedException alreadyClosed(RuntimeException npe) {
>     if (npe == null || this.buffers == null) { // buffers == null if input closed!
> return new AlreadyClosedException("Already closed: " + this);
> }
> throw npe;
>   }
>
> (this would need the same change in all MemorySegmentIndexInput in a
> similar way).
>
> This would keep the NPE on wrong usage, but in the case of a closed
> ByteBufferIndexInput / MemorySegmentIndexInput it would throw plain
> AlreadyClosedEx.
>
> I can provide a PR, but give me a week, I am very busy.
>
> Uwe
>
> Am 21.10.2023 um 10:01 schrieb Uwe Schindler:
>> Hi,
>>
>> please don't add the NPE here as cause (except for debugging). The
>> NPE is only catched to NOT add extra checks in the highly performance
>> sensitive code. Actually the NPE is catched to detect the case where
>> the bytebuffer was already unset to trigger the already closed. The
>> code uses setting the buffers to NULL to signal cause, but it does
>> NOT add a NULL check everywhere. This allows Hotspot to compile this
>> code without any bounds checks and signal the AlreadyClosedException
>> only when a NPE happens. Adding the NPE as cause would bring
>> confusion to end user, as we only want to tell that IndexInput was
>> closed, but the NPE should be kept behind scenes as it would be a
>> support nightmare ("your code has no good null checks, it is
>> broken"). The NPE is a signal here, not the cause.
>>
>> I think the issue you have seen may be cause by passing a NULL
>> parameter to one of the methods like a float array to readFloats().
>> This is not detected (P.S.: this also affects MemorySegmentIndexInput).
>>
>> I can looks at the code to figure out how to better detect the NPE
>> when parameters of methods are NULL, but no way to add the cause
>> here. I would say: If you have to debug, do it temporarily or ose
>> another dircetory impl.
>>
>> Uwe
>>
>> Am 20.10.2023 um 21:20 schrieb Chris Hostetter:
>>> FWIW: The choice to ignore the original exception goes back to here...
>>>
>>> https://issues.apache.org/jira/browse/LUCENE-3588
>>>
>>> ...circa 2011, where it was focused on catching NPE and throwing
>>> AlreadyClosedException instead, w/o any particular discussion as to
>>> why to
>>> throw away the original NPE.
>>>
>>> If i had to guess it's simply because at that time
>>> AlreadyClosedException
>>> didn't support wrapping any other Throwable.  That wasn't added until
>>> LUCENE-5958 (circa 2014) which was focused on making sure "tragic"
>>> errors
>>> kept a record of what caused the tragedy, and then include that as the
>>> 'cause' of the AlreadyClosedExceptions throw by 'ensureOpen()'
>>>
>>> There didn't seem to be any discussion at that time about reviewing
>>> other
>>> code that might be throwing AlreadyClosedException from a 'catch' block
>>> that could also be updated to include the cause.
>>>
>>> I'd say open a PR to review & update all code that results in
>>> AlreadyClosedException originating from a catch block?
>>>
>>>
>>>
>>> : Date: Tue, 17 Oct 2023 11:24:03 -0400
>>> : From: Michael Sokolov <msokolov@gmail.com>
>>> : Reply-To: dev@lucene.apache.org
>>> : To: Lucene Dev <dev@lucene.apache.org>
>>> : Subject: ByteBufferIndexInput.alreadyClosed creates an exception
>>> that doesn't
>>> :     track its cause
>>> :
>>> : I was messing around with something that was resulting in
>>> : AlreadyClosedException being thrown and I noticed that we weren't
>>> : tracking the exception that caused it. I found this in
>>> : ByteBufferIndexInput:
>>> :
>>> :    // the unused parameter is just to silence javac about unused
>>> variables
>>> :    AlreadyClosedException alreadyClosed(RuntimeException unused) {
>>> : -    return new AlreadyClosedException("Already closed: " + this);
>>> : +    return new AlreadyClosedException("Already closed: " + this,
>>> unused);
>>> :    }
>>> :
>>> : and added the cause there, which helped me find and fix my wicked
>>> : ways. Is there a reason we decided not to wrap the "unused"
>>> : RuntimeException there?
>>> :
>>> : ---------------------------------------------------------------------
>>> : To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>>> : For additional commands, e-mail: dev-help@lucene.apache.org
>>> :
>>> :
>>>
>>> -Hoss
>>> http://www.lucidworks.com/
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>>> For additional commands, e-mail: dev-help@lucene.apache.org
>>>
> --
> Uwe Schindler
> Achterdiek 19, D-28357 Bremen
> https://www.thetaphi.de
> eMail:uwe@thetaphi.de

--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail:uwe@thetaphi.de
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Uwe: In your PR, you should add these details to the javadocs of
ByteBufferIndexInput.alreadyClosed(), so future code spelunkers understand
the choice being made here is intentional :)

: please don't add the NPE here as cause (except for debugging). The NPE is only
: catched to NOT add extra checks in the highly performance sensitive code.
: Actually the NPE is catched to detect the case where the bytebuffer was
: already unset to trigger the already closed. The code uses setting the buffers
: to NULL to signal cause, but it does NOT add a NULL check everywhere. This
: allows Hotspot to compile this code without any bounds checks and signal the
: AlreadyClosedException only when a NPE happens. Adding the NPE as cause would



-Hoss
http://www.lucidworks.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Thanks for digging into this. I do think it will be helpful for
developers that blithely access the IndexInput from multiple threads
:)

On Sat, Oct 21, 2023 at 3:53?PM Chris Hostetter
<hossman_lucene@fucit.org> wrote:
>
>
> Uwe: In your PR, you should add these details to the javadocs of
> ByteBufferIndexInput.alreadyClosed(), so future code spelunkers understand
> the choice being made here is intentional :)
>
> : please don't add the NPE here as cause (except for debugging). The NPE is only
> : catched to NOT add extra checks in the highly performance sensitive code.
> : Actually the NPE is catched to detect the case where the bytebuffer was
> : already unset to trigger the already closed. The code uses setting the buffers
> : to NULL to signal cause, but it does NOT add a NULL check everywhere. This
> : allows Hotspot to compile this code without any bounds checks and signal the
> : AlreadyClosedException only when a NPE happens. Adding the NPE as cause would
>
>
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Hi,

I added long comment in the PR for all variants of that code (there are
actually 4 variants because we have 4 implementations).

Uwe

Am 21.10.2023 um 21:53 schrieb Chris Hostetter:
> Uwe: In your PR, you should add these details to the javadocs of
> ByteBufferIndexInput.alreadyClosed(), so future code spelunkers understand
> the choice being made here is intentional :)
>
> : please don't add the NPE here as cause (except for debugging). The NPE is only
> : catched to NOT add extra checks in the highly performance sensitive code.
> : Actually the NPE is catched to detect the case where the bytebuffer was
> : already unset to trigger the already closed. The code uses setting the buffers
> : to NULL to signal cause, but it does NOT add a NULL check everywhere. This
> : allows Hotspot to compile this code without any bounds checks and signal the
> : AlreadyClosedException only when a NPE happens. Adding the NPE as cause would
>
>
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail: uwe@thetaphi.de


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Please read my other comments and the PR. The PR filters the cause of
the NPE, if the NPE is caused by inernals of MMapDirectory it won't be
exposed to anybody.

If you use it in multiple threads and acidentally close one of the
indexinputs, AlreadyClosedException is the only correct exception. Any
cause like an internal signalling NPE is not useful and helps nothing.
The PR explains this, so we won't add the NPE as cause. If the NPE is
coming from outside MMapDircetory, it will be rethrown so you see it.

I will merge the PR in a moment.

Uwe

Am 22.10.2023 um 01:37 schrieb Michael Sokolov:
> Thanks for digging into this. I do think it will be helpful for
> developers that blithely access the IndexInput from multiple threads
> :)
>
> On Sat, Oct 21, 2023 at 3:53?PM Chris Hostetter
> <hossman_lucene@fucit.org> wrote:
>>
>> Uwe: In your PR, you should add these details to the javadocs of
>> ByteBufferIndexInput.alreadyClosed(), so future code spelunkers understand
>> the choice being made here is intentional :)
>>
>> : please don't add the NPE here as cause (except for debugging). The NPE is only
>> : catched to NOT add extra checks in the highly performance sensitive code.
>> : Actually the NPE is catched to detect the case where the bytebuffer was
>> : already unset to trigger the already closed. The code uses setting the buffers
>> : to NULL to signal cause, but it does NOT add a NULL check everywhere. This
>> : allows Hotspot to compile this code without any bounds checks and signal the
>> : AlreadyClosedException only when a NPE happens. Adding the NPE as cause would
>>
>>
>>
>> -Hoss
>> http://www.lucidworks.com/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail: uwe@thetaphi.de


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: ByteBufferIndexInput.alreadyClosed creates an exception that doesn't track its cause [ In reply to ]
Thanks, Uwe. The underlying exception in my situation was caused by
curFloatBufferViews being allocated and used before it was fully
populated. So I think it was an NPE, yes. I'll check your PR to see if
it would have hidden this?

On Sun, Oct 22, 2023 at 4:57?AM Uwe Schindler <uwe@thetaphi.de> wrote:
>
> Please read my other comments and the PR. The PR filters the cause of
> the NPE, if the NPE is caused by inernals of MMapDirectory it won't be
> exposed to anybody.
>
> If you use it in multiple threads and acidentally close one of the
> indexinputs, AlreadyClosedException is the only correct exception. Any
> cause like an internal signalling NPE is not useful and helps nothing.
> The PR explains this, so we won't add the NPE as cause. If the NPE is
> coming from outside MMapDircetory, it will be rethrown so you see it.
>
> I will merge the PR in a moment.
>
> Uwe
>
> Am 22.10.2023 um 01:37 schrieb Michael Sokolov:
> > Thanks for digging into this. I do think it will be helpful for
> > developers that blithely access the IndexInput from multiple threads
> > :)
> >
> > On Sat, Oct 21, 2023 at 3:53?PM Chris Hostetter
> > <hossman_lucene@fucit.org> wrote:
> >>
> >> Uwe: In your PR, you should add these details to the javadocs of
> >> ByteBufferIndexInput.alreadyClosed(), so future code spelunkers understand
> >> the choice being made here is intentional :)
> >>
> >> : please don't add the NPE here as cause (except for debugging). The NPE is only
> >> : catched to NOT add extra checks in the highly performance sensitive code.
> >> : Actually the NPE is catched to detect the case where the bytebuffer was
> >> : already unset to trigger the already closed. The code uses setting the buffers
> >> : to NULL to signal cause, but it does NOT add a NULL check everywhere. This
> >> : allows Hotspot to compile this code without any bounds checks and signal the
> >> : AlreadyClosedException only when a NPE happens. Adding the NPE as cause would
> >>
> >>
> >>
> >> -Hoss
> >> http://www.lucidworks.com/
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> >> For additional commands, e-mail: dev-help@lucene.apache.org
> >>
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: dev-help@lucene.apache.org
> >
> --
> Uwe Schindler
> Achterdiek 19, D-28357 Bremen
> https://www.thetaphi.de
> eMail: uwe@thetaphi.de
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>

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