Issue #372 (Data race in HttpReceiverOverHTTP2)

Fixed by copying the buffer passed to onData().
This commit is contained in:
Simone Bordet 2016-02-29 14:02:17 +01:00
parent aadfae936c
commit aa6de825b7
2 changed files with 42 additions and 4 deletions

View File

@ -19,6 +19,7 @@
package org.eclipse.jetty.http2.client.http; package org.eclipse.jetty.http2.client.http;
import java.io.IOException; import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.Locale; import java.util.Locale;
import org.eclipse.jetty.client.HttpChannel; import org.eclipse.jetty.client.HttpChannel;
@ -34,6 +35,8 @@ import org.eclipse.jetty.http2.frames.DataFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.PushPromiseFrame; import org.eclipse.jetty.http2.frames.PushPromiseFrame;
import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.http2.frames.ResetFrame;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Callback;
public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listener public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listener
@ -95,7 +98,41 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen
return; return;
} }
if (responseContent(exchange, frame.getData(), callback)) // We must copy the data since we do not know when the
// application will consume the bytes and the parsing
// will continue as soon as this method returns, eventually
// leading to reusing the underlying buffer for more reads.
ByteBufferPool byteBufferPool = getHttpDestination().getHttpClient().getByteBufferPool();
ByteBuffer original = frame.getData();
int length = original.remaining();
final ByteBuffer copy = byteBufferPool.acquire(length, original.isDirect());
BufferUtil.clearToFill(copy);
copy.put(original).flip();
Callback delegate = new Callback()
{
@Override
public boolean isNonBlocking()
{
return callback.isNonBlocking();
}
@Override
public void succeeded()
{
byteBufferPool.release(copy);
callback.succeeded();
}
@Override
public void failed(Throwable x)
{
byteBufferPool.release(copy);
callback.failed(x);
}
};
if (responseContent(exchange, copy, delegate))
{ {
if (frame.isEndStream()) if (frame.isEndStream())
responseSuccess(exchange); responseSuccess(exchange);

View File

@ -164,9 +164,10 @@ public class HttpChannelOverHTTP2 extends HttpChannel
public Runnable requestContent(DataFrame frame, final Callback callback) public Runnable requestContent(DataFrame frame, final Callback callback)
{ {
// We must copy the data since we do not know when the // We must copy the data since we do not know when the
// application will consume its bytes (we queue them by // application will consume the bytes (we queue them by
// calling onContent()), and we cannot stop the parsing // calling onContent()), and the parsing will continue
// since there may be frames for other streams. // as soon as this method returns, eventually leading
// to reusing the underlying buffer for more reads.
final ByteBufferPool byteBufferPool = getByteBufferPool(); final ByteBufferPool byteBufferPool = getByteBufferPool();
ByteBuffer original = frame.getData(); ByteBuffer original = frame.getData();
int length = original.remaining(); int length = original.remaining();