NightOwl888 commented on code in PR #1138:
URL: https://github.com/apache/lucenenet/pull/1138#discussion_r1986189945
##########
src/Lucene.Net/Store/MMapDirectory.cs:
##########
@@ -330,15 +330,15 @@ internal virtual ByteBuffer[] Map(MMapIndexInput input,
FileStream fc, long offs
// LUCENENET: We get an UnauthorizedAccessException if we
create a 0 byte file at the end of the range.
// See: https://stackoverflow.com/a/5501331
- // We can fix this by moving back 1 byte on the offset if the
bufSize is 0.
- int adjust = 0;
- if (bufSize == 0 && bufNr == (nrBuffers - 1) && (offset +
bufferStart) > 0)
+ // We can fix this by using an empty ByteBuffer if the buffer
size is 0.
+ if (bufSize == 0 && bufNr == (nrBuffers - 1))
{
- adjust = 1;
+ buffers[bufNr] = ByteBuffer.Allocate(0).AsReadOnlyBuffer();
+ break;
}
buffers[bufNr] = input.memoryMappedFile.CreateViewByteBuffer(
- offset: (offset + bufferStart) - adjust,
+ offset: offset + bufferStart,
Review Comment:
Note that offset could overflow `long`. But I am not sure whether it is
worth the cost of performance to do an extra check just to throw an
`ArgumentOutOfRangeException` with a more sensible message than "less than
zero". Microsoft sometimes throws "less than zero" exceptions for overflow
cases to reduce the number of guard operations.
##########
src/Lucene.Net/Store/MMapDirectory.cs:
##########
@@ -314,7 +314,7 @@ internal virtual ByteBuffer[] Map(MMapIndexInput input,
FileStream fc, long offs
input.memoryMappedFile = MemoryMappedFile.CreateFromFile(
fileStream: fc,
mapName: null,
- capacity: length,
+ capacity: 0, // use the full length of the file
Review Comment:
Given how unpredictable MemoryMappedFile behavior is across platforms, I am
not sure we should only rely on the documentation for this. There were several
problems with MemoryMappedFile tests on J2N when using Xamarin Android (Mono),
but fortunately none of them affected Lucene.NET. The behavior seems to be
broken on Mono: https://stackoverflow.com/q/9895384
Let's just use `length` here to be sure we have the full file length instead
of falling back on the specific implementation, which may or may not behave as
documented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
