Skip to content

Commit

Permalink
fix HttpChannelOverHTTP2 recycling order to make sure no demanding co…
Browse files Browse the repository at this point in the history
…ntent can be left pending

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
  • Loading branch information
lorban committed May 19, 2021
1 parent 65ef393 commit 6ed64e1
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,13 @@ public HttpTransportOverHTTP2 getHttpTransport()
@Override
public void recycle()
{
super.recycle();
getHttpTransport().recycle();
_expect100Continue = false;
_delayedUntilContent = false;
// The content demander must be the very last thing to be recycled
// to make sure any pending demanding content gets cleared off.
_contentDemander.recycle();
super.recycle();
getHttpTransport().recycle();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,15 @@ public HttpInput(HttpChannelState state)

public void recycle()
{
if (LOG.isDebugEnabled())
LOG.debug("recycle {}", this);
reopen();
}

public void reopen()
{
try (AutoLock lock = _contentProducer.lock())
{
if (LOG.isDebugEnabled())
LOG.debug("reopen {}", this);
LOG.debug("recycle/reopen {}", this);
_blockingContentProducer.recycle();
_contentProducer = _blockingContentProducer;
_consumedEof = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.http.client;

import java.io.BufferedReader;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.util.BytesRequestContent;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;

public class RequestReaderTest extends AbstractTest<TransportScenario>
{
@Override
public void init(Transport transport) throws IOException
{
setScenario(new TransportScenario(transport));
}

@ParameterizedTest
@ArgumentsSource(TransportProvider.class)
public void testRecyclingWhenUsingReader(Transport transport) throws Exception
{
init(transport);
scenario.start(new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
// Must be a Reader and not an InputStream.
BufferedReader br = request.getReader();
while (true)
{
int b = br.read();
if (b == -1)
break;
}
baseRequest.setHandled(true);
}
}, client -> {});

ContentResponse response1 = scenario.client.newRequest("localhost", scenario.server.getURI().getPort())
.scheme(scenario.server.getURI().getScheme())
.method("POST")
.timeout(5, TimeUnit.SECONDS)
.body(new BytesRequestContent(new byte[512]))
.send();
assertThat(response1.getStatus(), is(HttpStatus.OK_200));

// Send a 2nd request to make sure recycling works.
ContentResponse response2 = scenario.client.newRequest("localhost", scenario.server.getURI().getPort())
.scheme(scenario.server.getURI().getScheme())
.method("POST")
.timeout(5, TimeUnit.SECONDS)
.body(new BytesRequestContent(new byte[512]))
.send();
assertThat(response2.getStatus(), is(HttpStatus.OK_200));
}
}

0 comments on commit 6ed64e1

Please sign in to comment.