Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3340: javalib MappedByteBuffer now handles 0 byte ranges #3360

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 37 additions & 7 deletions javalib/src/main/scala/java/nio/MappedByteBufferData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,44 @@ private[nio] class MappedByteBufferData(
private[nio] val windowsMappingHandle: Option[Handle]
) {

/* Create an "empty" instance for the special case of size == 0.
* This removes that complexity from the execution paths of the
* more frequently used size > 0 case.
*
* Keep the nasty null confined to this file, so caller does not
* need to know about it.
*
* Execution should never reach update() or apply() (bb.get()).
* Code earlier in the execution chain should have detected and rejected
* an attempt to access an empty MappedByteBufferData instance.
* Have those two methods return "reasonable" values just in case.
* Could have thrown an Exception. Fielder's choice.
*
* Since it is never called, the return value for apply() is just to
* keep the compiler happy; it can be any Byte, zero seemed to make the
* most sense. Fielder's choice redux.
*/
def this() = {
this(MapMode.READ_ONLY, null, 0, None)
def force(): Unit = () // do nothing
def update(index: Int, value: Byte): Unit = () // do nothing
def apply(index: Int): Byte = 0 // Should never reach here
}

// Finalization. Unmapping is done on garbage collection, like on JVM.
private val selfWeakReference = new WeakReference(this)
new MappedByteBufferFinalizer(
selfWeakReference,
ptr,
length,
windowsMappingHandle
)
// private val selfWeakReference = new WeakReference(this)

if (ptr != null) {
// Finalization. Unmapping is done on garbage collection, like on JVM.
val selfWeakReference = new WeakReference(this)

new MappedByteBufferFinalizer(
selfWeakReference,
ptr,
length,
windowsMappingHandle
)
}

def force(): Unit = {
if (mode eq MapMode.READ_WRITE) {
Expand Down
37 changes: 34 additions & 3 deletions javalib/src/main/scala/java/nio/MappedByteBufferImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -323,16 +323,47 @@ private[nio] object MappedByteBufferImpl {
new MappedByteBufferData(mode, ptr, size, None)
}

private def mapData(
position: Long,
size: Int,
fd: FileDescriptor,
mode: MapMode
): MappedByteBufferData = {

if (size > 0) {
if (isWindows) mapWindows(position, size, fd, mode)
else mapUnix(position, size, fd, mode)
} else {
/* Issue #3340
* JVM silently succeeds on MappedByteBuffer creation and
* throws "IndexOutOfBoundsException" on access; get or put.
*
* Create and use an "empty" MappedByteBuffer so that Scala Native
* matches the JVM behavior.
*
* POSIX and most (all?) unix-like systems explicitly do not
* allow mapping zero bytes and mapUnix() will throw an Exception.
*
* On Windows, a request to map zero bytes causes the entire
* file to be mapped. At the least, expensive in I/O and memory
* for bytes which will never be used. The call to MapViewOfFile()
* in mapWindows() may or may not use the same semantics. Someone
* with Windows skills would have to check. Knowing the zero size,
* it is easier to match the JDK by creating an empty
* MappedByteBufferData on the Windows branch also.
*/
new MappedByteBufferData()
}
}

def apply(
mode: MapMode,
position: Long,
size: Int,
fd: FileDescriptor
): MappedByteBufferImpl = {

val mappedData =
if (isWindows) mapWindows(position, size, fd, mode)
else mapUnix(position, size, fd, mode)
val mappedData = mapData(position, size, fd, mode)

new MappedByteBufferImpl(
mappedData.length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,39 @@ class FileChannelTest {
}
}

// Issue #3340
@Test def mapMethodMapZeroBytes(): Unit = {
withTemporaryDirectory { dir =>
val data = s"ABCDEF"
val dataBytes = data.getBytes("UTF-8")

val f = dir.resolve("mapZeroBytes.txt")
Files.write(f, dataBytes)

val lines = Files.readAllLines(f)
assertEquals("lines size", 1, lines.size())
assertEquals("lines content", data, lines.get(0))

val channel = FileChannel.open(
f,
StandardOpenOption.READ,
StandardOpenOption.WRITE
)

try {
val mappedChan = channel.map(FileChannel.MapMode.READ_WRITE, 0, 0)

assertThrows(
classOf[java.lang.IndexOutOfBoundsException],
mappedChan.get(0)
)

} finally {
channel.close()
}
}
}

@Test def cannotTruncateChannelUsingNegativeSize(): Unit = {
withTemporaryDirectory { dir =>
val f = dir.resolve("negativeSize.txt")
Expand Down