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

Issue #12695 - don't return ByteBuffer from .toMappedBuffer if FS reports as UnsupportedOperationException. #12696

Merged
merged 7 commits into from
Jan 15, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -262,17 +262,25 @@ public static Response parseResponse(Input input) throws IOException
return parseResponse(input, false);
}

public static Response parseResponse(Input input, Response response) throws IOException
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While restoring FileBufferedResponseHandlerTest (from earlier versions of Jetty) this method in HttpTester was also restored.

{
return parseResponse(input, response, false);
}

public static Response parseResponse(Input input, boolean head) throws IOException
{
Response response;
return parseResponse(input, new Response(), head);
}

public static Response parseResponse(Input input, Response response, boolean head) throws IOException
{
HttpParser parser = input.takeHttpParser();
if (parser != null)
{
response = (Response)parser.getHandler();
}
else
{
response = new Response();
parser = new HttpParser(response);
}
parser.setHeadResponse(head);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ private SingleBufferFileMappedHttpContent(HttpContent content) throws IOExceptio
if (path == null)
throw new IOException("Cannot memory map Content whose Resource is not backed by a Path: " + content.getResource());
_buffer = BufferUtil.toMappedBuffer(path);
if (_buffer == null)
throw new IOException("Cannot memory map Content (not supported by underlying FileSystem): " + content.getResource());
_contentLength = new HttpField(HttpHeader.CONTENT_LENGTH, Integer.toString(_buffer.remaining()));
_lastModified = content.getLastModified();
_lastModifiedInstant = content.getLastModifiedInstant();
Expand Down Expand Up @@ -176,6 +178,8 @@ private MultiBufferFileMappedHttpContent(HttpContent content, int maxBufferSize)
{
long len = Math.min(contentLength - currentPos, maxBufferSize);
_buffers[i] = BufferUtil.toMappedBuffer(path, currentPos, len);
if (_buffers[i] == null)
throw new IOException("Cannot memory map Content (not supported by underlying FileSystem): " + content.getResource());
currentPos += len;
total += _buffers[i].remaining();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.nio.Buffer;
import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -1166,19 +1167,48 @@ public static ByteBuffer toDirectBuffer(String s, Charset charset)
return buf;
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param path the path to map
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel}
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Path path) throws IOException
{
return toMappedBuffer(path, 0, Files.size(path));
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param filePath the path to map
* @param pos the position to start from
* @param len the length to support
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel}
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Path filePath, long pos, long len) throws IOException
{
try (FileChannel channel = FileChannel.open(filePath, StandardOpenOption.READ))
{
return channel.map(MapMode.READ_ONLY, pos, len);
}
catch (UnsupportedEncodingException e)
{
if (LOG.isTraceEnabled())
LOG.trace("Ignored exception", e);
return null;
}
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param resource the resource to map
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel} (usually due to Resource not supporting memory mapped files)
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Resource resource) throws IOException
{
Path path = resource.getPath();
Expand All @@ -1187,6 +1217,15 @@ public static ByteBuffer toMappedBuffer(Resource resource) throws IOException
return toMappedBuffer(path);
}

/**
* Attempt to return a ByteBuffer created via {@link FileChannel#map} call.
*
* @param resource the resource to map
* @param pos the position to start from
* @param len the length to support
* @return the mapped bytebuffer, or null if unable to map the {@link FileChannel} (usually due to Resource not supporting memory mapped files)
* @throws IOException if unable to open mapped byte buffer
*/
public static ByteBuffer toMappedBuffer(Resource resource, long pos, long len) throws IOException
{
Path path = resource.getPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.IOException;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.SeekableByteChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
Expand Down Expand Up @@ -52,6 +53,9 @@ public class FileBufferedResponseHandler extends BufferedResponseHandler
{
private static final Logger LOG = LoggerFactory.getLogger(FileBufferedResponseHandler.class);

private static final int DEFAULT_BUFFER_SIZE = 64 * 1024;
private int _bufferSize = DEFAULT_BUFFER_SIZE;
private boolean _useFileMapping = true;
private Path _tempDir = new File(System.getProperty("java.io.tmpdir")).toPath();

public Path getTempDir()
Expand All @@ -64,6 +68,26 @@ public void setTempDir(Path tempDir)
_tempDir = Objects.requireNonNull(tempDir);
}

public boolean isUseFileMapping()
{
return _useFileMapping;
}

public void setUseFileMapping(boolean useFileMapping)
{
this._useFileMapping = useFileMapping;
}

public int getBufferSize()
{
return _bufferSize;
}

public void setBufferSize(int bufferSize)
{
this._bufferSize = bufferSize;
}

@Override
protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, Interceptor interceptor)
{
Expand All @@ -72,8 +96,6 @@ protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, In

class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
{
private static final int MAX_MAPPED_BUFFER_SIZE = Integer.MAX_VALUE / 2;

private final Interceptor _next;
private final HttpChannel _channel;
private Boolean _aggregating;
Expand Down Expand Up @@ -193,41 +215,154 @@ private void commit(Callback callback)
}

// Create an iterating callback to do the writing
IteratingCallback icb = new IteratingCallback()
try
{
SendFileCallback sfcb = new SendFileCallback(this, _filePath, getBufferSize(), callback);
sfcb.setUseFileMapping(isUseFileMapping());
sfcb.iterate();
}
catch (IOException e)
{
private final long fileLength = _filePath.toFile().length();
private long _pos = 0;
private boolean _last = false;
callback.failed(e);
}
}
}

// TODO: can this be made generic enough to put into jetty-io somewhere?
joakime marked this conversation as resolved.
Show resolved Hide resolved
private static class SendFileCallback extends IteratingCallback
{
private static final int MAX_BUFFER_SIZE = Integer.MAX_VALUE / 2;
private final Path _filePath;
private final long _fileLength;
private final FileBufferedInterceptor _interceptor;
private final Callback _callback;
private final int _bufferSize;
private long _pos = 0;
private boolean _last = false;
private Mode _mode = Mode.DISCOVER;

enum Mode
{
DISCOVER,
joakime marked this conversation as resolved.
Show resolved Hide resolved
MAPPED,
READ
}

@Override
protected Action process() throws Exception
public SendFileCallback(FileBufferedInterceptor interceptor, Path filePath, int bufferSize, Callback callback) throws IOException
{
_filePath = filePath;
_fileLength = Files.size(filePath);
_interceptor = interceptor;
_callback = callback;
_bufferSize = bufferSize;
}

public void setUseFileMapping(boolean useFileMapping)
{
if (!useFileMapping)
_mode = Mode.READ; // don't even attempt file mapping
else
_mode = Mode.DISCOVER; // attempt file mapping first
}

@Override
protected Action process() throws Exception
{
if (_last)
return Action.SUCCEEDED;

long len = Math.min(MAX_BUFFER_SIZE, _fileLength - _pos);
ByteBuffer buffer = readByteBuffer(_filePath, _pos, len);
if (buffer == null)
{
buffer = BufferUtil.EMPTY_BUFFER;
_last = true;
}
else
{
_last = (_pos + buffer.remaining() == _fileLength);
}
int read = buffer.remaining();
_interceptor.getNextInterceptor().write(buffer, _last, this);
_pos += read;
return Action.SCHEDULED;
}

@Override
protected void onCompleteSuccess()
{
_interceptor.dispose();
_callback.succeeded();
}

@Override
protected void onFailure(Throwable cause)
{
_interceptor.dispose();
_callback.failed(cause);
}

/**
* Read the ByteBuffer from the path.
*
* @param path the path to read from
* @param pos the position in the file to start from
* @param len the length of the buffer to use for memory mapped mode
* @return the buffer read, or null if no buffer has been read (such as being at EOF)
* @throws IOException if unable to read from the path
*/
private ByteBuffer readByteBuffer(Path path, long pos, long len) throws IOException
{
return switch (_mode)
{
case DISCOVER ->
{
if (_last)
return Action.SUCCEEDED;

long len = Math.min(MAX_MAPPED_BUFFER_SIZE, fileLength - _pos);
_last = (_pos + len == fileLength);
ByteBuffer buffer = BufferUtil.toMappedBuffer(_filePath, _pos, len);
getNextInterceptor().write(buffer, _last, this);
_pos += len;
return Action.SCHEDULED;
ByteBuffer buffer = toMapped(path, pos, len);
if (buffer == null)
{
// if we reached here, then file mapped byte buffers is not supported.
// we fall back to using traditional I/O instead.
buffer = toRead(path, pos);
}
yield buffer;
}

@Override
protected void onCompleteSuccess()
case MAPPED ->
{
dispose();
callback.succeeded();
yield toMapped(path, pos, len);
}

@Override
protected void onFailure(Throwable cause)
case READ ->
{
dispose();
callback.failed(cause);
yield toRead(path, pos);
}
};
icb.iterate();
}

private ByteBuffer toMapped(Path path, long pos, long len) throws IOException
{
if (pos > _fileLength)
{
// attempt to read past end of file, consider this an EOF
return null;
}
ByteBuffer buffer = BufferUtil.toMappedBuffer(path, pos, len);
if (buffer != null)
_mode = Mode.MAPPED;
return buffer;
}

private ByteBuffer toRead(Path path, long pos) throws IOException
joakime marked this conversation as resolved.
Show resolved Hide resolved
{
try (SeekableByteChannel channel = Files.newByteChannel(path))
{
_mode = Mode.READ;
channel.position(pos);
ByteBuffer buffer = ByteBuffer.allocateDirect(_bufferSize);
int read = channel.read(buffer);
if (read == -1)
return null; // indicating EOF
buffer.flip();
return buffer;
}
}
}
}
Loading
Loading