-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 6 commits
308cdf4
d1a8929
459700c
9dc366e
238569c
a7996c5
12e4a47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,6 +122,21 @@ class Sized extends Wrapper | |
private final boolean _direct; | ||
private final int _size; | ||
|
||
/** | ||
* Get a sized pool for any arbitrary ByteBufferPool. | ||
* | ||
* @param pool the pool | ||
* @param direct {@code true} for direct buffers (if wrapping pool) | ||
* @param size The specified size in bytes of the buffer, or -1 for a default | ||
* @return the pool as a sized pool | ||
*/ | ||
public static ByteBufferPool.Sized as(ByteBufferPool pool, boolean direct, int size) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I like this new helper as it takes parameters that could be ignored without any hint. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
{ | ||
if (pool instanceof Sized sized) | ||
return sized; | ||
return new Sized(pool, direct, size); | ||
} | ||
|
||
/** | ||
* Create a sized pool for non direct buffers of a default size from a wrapped pool. | ||
* @param wrapped The actual {@link ByteBufferPool} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,10 +23,11 @@ | |
import java.util.Objects; | ||
|
||
import org.eclipse.jetty.ee9.nested.HttpOutput.Interceptor; | ||
import org.eclipse.jetty.io.ByteBufferPool; | ||
import org.eclipse.jetty.io.Content; | ||
import org.eclipse.jetty.util.BufferUtil; | ||
import org.eclipse.jetty.util.Callback; | ||
import org.eclipse.jetty.util.IO; | ||
import org.eclipse.jetty.util.IteratingCallback; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -52,6 +53,8 @@ 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 Path _tempDir = new File(System.getProperty("java.io.tmpdir")).toPath(); | ||
|
||
public Path getTempDir() | ||
|
@@ -64,6 +67,16 @@ public void setTempDir(Path tempDir) | |
_tempDir = Objects.requireNonNull(tempDir); | ||
} | ||
|
||
public int getBufferSize() | ||
{ | ||
return _bufferSize; | ||
} | ||
|
||
public void setBufferSize(int bufferSize) | ||
{ | ||
this._bufferSize = bufferSize; | ||
} | ||
|
||
@Override | ||
protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, Interceptor interceptor) | ||
{ | ||
|
@@ -72,8 +85,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; | ||
|
@@ -193,41 +204,27 @@ private void commit(Callback callback) | |
} | ||
|
||
// Create an iterating callback to do the writing | ||
IteratingCallback icb = new IteratingCallback() | ||
{ | ||
private final long fileLength = _filePath.toFile().length(); | ||
private long _pos = 0; | ||
private boolean _last = false; | ||
ByteBufferPool.Sized sizedPool = ByteBufferPool.Sized.as(getServer().getByteBufferPool(), true, getBufferSize()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd rather see a new instance of |
||
Content.Source source = Content.Source.from(sizedPool, _filePath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Supporting mmap from Content.Source.from(path) is still possible, but in a future PR. |
||
Content.Sink sink = new InterceptorSink(getNextInterceptor()); | ||
Callback disposer = Callback.from(callback, this::dispose); | ||
Content.copy(source, sink, disposer); | ||
} | ||
} | ||
|
||
@Override | ||
protected Action process() throws Exception | ||
{ | ||
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; | ||
} | ||
private static class InterceptorSink implements Content.Sink | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could inline this class as a simple lambda: Content.Sink sink = (last, byteBuffer, callback) -> _interceptor.write(last, byteBuffer, callback); |
||
{ | ||
private final HttpOutput.Interceptor _interceptor; | ||
|
||
@Override | ||
protected void onCompleteSuccess() | ||
{ | ||
dispose(); | ||
callback.succeeded(); | ||
} | ||
public InterceptorSink(HttpOutput.Interceptor interceptor) | ||
{ | ||
this._interceptor = interceptor; | ||
} | ||
|
||
@Override | ||
protected void onFailure(Throwable cause) | ||
{ | ||
dispose(); | ||
callback.failed(cause); | ||
} | ||
}; | ||
icb.iterate(); | ||
@Override | ||
public void write(boolean last, ByteBuffer byteBuffer, Callback callback) | ||
{ | ||
_interceptor.write(last, byteBuffer, callback); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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 inHttpTester
was also restored.