If the buffers are nonnull and the guard state is sane, it would have thrown the exception, like on incomplete views.
If buffers or guard is invalidated, the indexinput was closed. The state of curFloatBufferViews would then not matter.
Anyways: Indexinput is not allowed to be used by multiple threads, so it's a bug in your code. The code has state (position to read) and it is documented to be not thread safe.
Uwe
Am 22. Oktober 2023 14:37:06 MESZ schrieb Michael Sokolov <msokolov@gmail.com>:
>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
>
--
Uwe Schindler
Achterdiek 19, 28357 Bremen
https://www.thetaphi.de
If buffers or guard is invalidated, the indexinput was closed. The state of curFloatBufferViews would then not matter.
Anyways: Indexinput is not allowed to be used by multiple threads, so it's a bug in your code. The code has state (position to read) and it is documented to be not thread safe.
Uwe
Am 22. Oktober 2023 14:37:06 MESZ schrieb Michael Sokolov <msokolov@gmail.com>:
>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
>
--
Uwe Schindler
Achterdiek 19, 28357 Bremen
https://www.thetaphi.de