Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/copilot-setup-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ jobs:
distribution: 'temurin'
cache: maven

- name: Prepare custom Maven repository directory
run: mkdir -p .m2_repo

- name: Cache custom Maven repository
uses: actions/cache@v4
with:
path: .m2_repo
key: ${{ runner.os }}-m2-repo-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-m2-repo-

- name: Validate formatting configuration
run: mvn -B -T 2C -Dmaven.repo.local=.m2_repo formatter:validate impsort:check xml-format:xml-check

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.eclipse.rdf4j.common.io;

import java.io.Closeable;
import java.io.EOFException;
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
Expand All @@ -22,6 +23,7 @@
import java.nio.file.StandardOpenOption;
import java.util.EnumSet;
import java.util.Set;
import java.util.concurrent.TimeUnit;

/**
* File wrapper that protects against concurrent file closing events due to e.g. {@link Thread#interrupt() thread
Expand All @@ -45,6 +47,13 @@ public final class NioFile implements Closeable {

private volatile FileChannel fc;

/**
* Disable strict guards via system property to maintain legacy behavior without additional exceptions. Property:
* org.eclipse.rdf4j.common.io.niofile.disableStrictGuards (default: false)
*/
private static final boolean STRICT_GUARDS = !Boolean
.getBoolean("org.eclipse.rdf4j.common.io.niofile.disableStrictGuards");

private volatile boolean explictlyClosed;

/**
Expand Down Expand Up @@ -238,12 +247,24 @@ public long transferTo(long position, long count, WritableByteChannel target) th
* @throws IOException
*/
public int write(ByteBuffer buf, long offset) throws IOException {
final int startPosition = buf.position();
while (true) {
try {
return fc.write(buf, offset);
// Ensure the entire buffer is written, even if the underlying channel performs a partial write
while (buf.hasRemaining()) {
long position = offset + (buf.position() - startPosition);
int n = fc.write(buf, position);
if (n == 0) {
// Avoid tight spin in pathological cases: reattempt write
// FileChannel positional writes may occasionally return 0 without progress
continue;
}
}
return buf.position() - startPosition;
} catch (ClosedByInterruptException e) {
throw e;
} catch (ClosedChannelException e) {
// Preserve already-consumed bytes on retry
reopen(e);
}
}
Expand All @@ -258,12 +279,29 @@ public int write(ByteBuffer buf, long offset) throws IOException {
* @throws IOException
*/
public int read(ByteBuffer buf, long offset) throws IOException {
final int startPosition = buf.position();
while (true) {
try {
return fc.read(buf, offset);
while (buf.hasRemaining()) {
long position = offset + (buf.position() - startPosition);
int n = fc.read(buf, position);
if (n < 0) {
// Preserve FileChannel contract: if no bytes were read and EOF is reached, return -1
if (buf.position() == startPosition) {
return -1;
}
break; // EOF after having read some bytes
}
if (n == 0) {
// Avoid tight spin; allow retry in case of transient 0-byte read
continue;
}
}
return buf.position() - startPosition;
} catch (ClosedByInterruptException e) {
throw e;
} catch (ClosedChannelException e) {
// Preserve already-consumed bytes on retry
reopen(e);
}
}
Expand All @@ -277,7 +315,10 @@ public int read(ByteBuffer buf, long offset) throws IOException {
* @throws IOException
*/
public void writeBytes(byte[] value, long offset) throws IOException {
write(ByteBuffer.wrap(value), offset);
int write = write(ByteBuffer.wrap(value), offset);
if (STRICT_GUARDS && write != value.length) {
throw new IOException("Incomplete writeBytes: expected " + value.length + ", wrote " + write);
}
}

/**
Expand All @@ -290,7 +331,10 @@ public void writeBytes(byte[] value, long offset) throws IOException {
*/
public byte[] readBytes(long offset, int length) throws IOException {
ByteBuffer buf = ByteBuffer.allocate(length);
read(buf, offset);
int read = read(buf, offset);
if (STRICT_GUARDS && read < length) {
throw new EOFException("Unexpected EOF in readBytes: expected " + length + ", read " + read);
}
return buf.array();
}

Expand Down Expand Up @@ -326,7 +370,10 @@ public byte readByte(long offset) throws IOException {
public void writeLong(long value, long offset) throws IOException {
ByteBuffer buf = ByteBuffer.allocate(8);
buf.putLong(0, value);
write(buf, offset);
int write = write(buf, offset);
if (STRICT_GUARDS && write != 8) {
throw new IOException("Incomplete writeLong: wrote " + write);
}
}

/**
Expand All @@ -338,7 +385,10 @@ public void writeLong(long value, long offset) throws IOException {
*/
public long readLong(long offset) throws IOException {
ByteBuffer buf = ByteBuffer.allocate(8);
read(buf, offset);
int read = read(buf, offset);
if (STRICT_GUARDS && read < 8) {
throw new EOFException("Unexpected EOF in readLong: read " + read);
}
return buf.getLong(0);
}

Expand All @@ -352,7 +402,10 @@ public long readLong(long offset) throws IOException {
public void writeInt(int value, long offset) throws IOException {
ByteBuffer buf = ByteBuffer.allocate(4);
buf.putInt(0, value);
write(buf, offset);
int write = write(buf, offset);
if (STRICT_GUARDS && write != 4) {
throw new IOException("Incomplete writeInt: wrote " + write);
}
}

/**
Expand All @@ -364,7 +417,10 @@ public void writeInt(int value, long offset) throws IOException {
*/
public int readInt(long offset) throws IOException {
ByteBuffer buf = ByteBuffer.allocate(4);
read(buf, offset);
int read = read(buf, offset);
if (STRICT_GUARDS && read < 4) {
throw new EOFException("Unexpected EOF in readInt: read " + read);
}
return buf.getInt(0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*******************************************************************************
* Copyright (c) 2025 Eclipse RDF4J contributors.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Distribution License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: BSD-3-Clause
*******************************************************************************/
package org.eclipse.rdf4j.common.io;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.File;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

/**
* Verifies the EOF contract for NioFile#read(ByteBuffer,long): when the underlying channel is at EOF and no bytes were
* read, the method must return -1 (EOF sentinel), not 0.
*/
public class NioFileEOFContractTest {

@TempDir
File tmp;

@Test
public void readReturnsMinusOneAtEofWhenNoBytesRead() throws Exception {
Path p = tmp.toPath().resolve("empty.dat");
Files.write(p, new byte[0]); // empty file

try (NioFile nf = new NioFile(p.toFile())) {
ByteBuffer buf = ByteBuffer.allocate(16);
int n = nf.read(buf, 0);
assertEquals(-1, n, "EOF sentinel -1 expected when no bytes were read");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*******************************************************************************
* Copyright (c) 2025 Eclipse RDF4J contributors.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Distribution License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: BSD-3-Clause
*******************************************************************************/
package org.eclipse.rdf4j.common.io;

import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.File;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

/**
* Regression test for heap guard misclassification: NioFile.read(ByteBuffer,long) must not preemptively throw
* IOException based solely on buffer size when the buffer is already allocated. Previously, a guard attempted to
* compare the requested read size to free heap and could throw before performing any IO, breaking legitimate large
* heap-buffer reads.
*/
public class NioFileLargeReadGuardRegressionTest {

@TempDir
File tmp;

@Test
public void largeHeapBufferReadDoesNotPreemptivelyThrow() throws Exception {
// Prepare a small file; the size/content are irrelevant to the regression
Path p = tmp.toPath().resolve("small.dat");
byte[] small = new byte[1024];
Files.write(p, small);

// Allocate a heap buffer just above the previous guard threshold (128MB)
final int size = 129 * 1024 * 1024; // 129MB
final ByteBuffer buf;
try {
buf = ByteBuffer.allocate(size);
} catch (OutOfMemoryError oom) {
// Not enough heap available in this environment to run the check; skip rather than fail
Assumptions.assumeTrue(false, "Insufficient heap to allocate 129MB test buffer");
return; // unreachable, but keeps compiler happy
}

// Reduce observed free heap below the requested read size so that a pre-check (if present) would misclassify
// this legitimate large buffer read as risky and throw. We allocate temporary blocks to lower free heap.
// If we cannot safely get below the threshold, skip the test to avoid flakiness across environments.
if (!saturateHeapToBelow(size)) {
Assumptions.assumeTrue(false, "Could not reduce free heap below threshold deterministically");
return;
}

try (NioFile nf = new NioFile(p.toFile())) {
int n = nf.read(buf, 0);
// Success is simply: no IOException thrown; value can be -1 (EOF) or >=0 bytes read
assertTrue(n >= -1, "read() returned unexpected value");
}
}

private static boolean saturateHeapToBelow(long bytes) {
final Runtime rt = Runtime.getRuntime();
final java.util.List<byte[]> blocks = new java.util.ArrayList<>();
final int block = 8 * 1024 * 1024; // 8MB steps to avoid big spikes
try {
for (int i = 0; i < 512; i++) { // cap allocations defensively
long alloc = rt.totalMemory() - rt.freeMemory();
long free = rt.maxMemory() - alloc;
if (free <= bytes) {
return true;
}
int size = (int) Math.min(block, Math.max(1, free - bytes));
blocks.add(new byte[size]);
}
} catch (OutOfMemoryError oom) {
// Best-effort: after OOM, the VM may still have reduced free; treat as inconclusive
}
long alloc = rt.totalMemory() - rt.freeMemory();
long free = rt.maxMemory() - alloc;
return free <= bytes;
}
}
Loading