Fixes #3512 - File descriptor is not released after zip file uploaded via jetty-client.

In case of multiple parts only the last iterator was closed.
Now, every part's iterator is closed.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2019-12-03 11:36:53 +01:00
parent db9ad2fcec
commit 9628ea3bc1
3 changed files with 103 additions and 14 deletions

View File

@ -26,6 +26,7 @@ import java.util.Iterator;
import org.eclipse.jetty.client.api.ContentProvider;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@ -217,16 +218,9 @@ public class HttpContent implements Callback, Closeable
@Override
public void close()
{
try
{
if (iterator instanceof Closeable)
((Closeable)iterator).close();
}
catch (Throwable x)
{
LOG.ignore(x);
}
IO.close((Closeable)iterator);
}
@Override

View File

@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@ -345,10 +346,16 @@ public class MultiPartContentProvider extends AbstractTypedContentProvider imple
if (iterator.hasNext())
return iterator.next();
++index;
if (index == parts.size())
state = State.LAST_BOUNDARY;
else
if (index < parts.size())
{
state = State.MIDDLE_BOUNDARY;
if (iterator instanceof Closeable)
IO.close((Closeable)iterator);
}
else
{
state = State.LAST_BOUNDARY;
}
break;
}
case MIDDLE_BOUNDARY:
@ -380,14 +387,14 @@ public class MultiPartContentProvider extends AbstractTypedContentProvider imple
@Override
public void succeeded()
{
if (iterator instanceof Callback)
if (state == State.CONTENT && iterator instanceof Callback)
((Callback)iterator).succeeded();
}
@Override
public void failed(Throwable x)
{
if (iterator instanceof Callback)
if (state == State.CONTENT && iterator instanceof Callback)
((Callback)iterator).failed(x);
}

View File

@ -20,6 +20,7 @@ package org.eclipse.jetty.client.util;
import java.io.BufferedWriter;
import java.io.ByteArrayInputStream;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
@ -31,10 +32,12 @@ import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.MultipartConfigElement;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@ -435,6 +438,46 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest
assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
}
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testEachPartIsClosed(Scenario scenario) throws Exception
{
String name1 = "field1";
String value1 = "value1";
String name2 = "field2";
String value2 = "value2";
start(scenario, new AbstractMultiPartHandler()
{
@Override
protected void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
Collection<Part> parts = request.getParts();
assertEquals(2, parts.size());
Iterator<Part> iterator = parts.iterator();
Part part1 = iterator.next();
assertEquals(name1, part1.getName());
assertEquals(value1, IO.toString(part1.getInputStream()));
Part part2 = iterator.next();
assertEquals(name2, part2.getName());
assertEquals(value2, IO.toString(part2.getInputStream()));
}
});
AtomicInteger closeCount = new AtomicInteger();
MultiPartContentProvider multiPart = new MultiPartContentProvider();
multiPart.addFieldPart(name1, new CloseableStringContentProvider(value1, closeCount::incrementAndGet), null);
multiPart.addFieldPart(name2, new CloseableStringContentProvider(value2, closeCount::incrementAndGet), null);
multiPart.close();
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.method(HttpMethod.POST)
.content(multiPart)
.send();
assertEquals(200, response.getStatus());
assertEquals(2, closeCount.get());
}
private abstract static class AbstractMultiPartHandler extends AbstractHandler
{
@Override
@ -448,4 +491,49 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest
protected abstract void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException;
}
private static class CloseableStringContentProvider extends StringContentProvider
{
private final Runnable closeFn;
private CloseableStringContentProvider(String content, Runnable closeFn)
{
super(content);
this.closeFn = closeFn;
}
@Override
public Iterator<ByteBuffer> iterator()
{
return new CloseableIterator<>(super.iterator());
}
private class CloseableIterator<T> implements Iterator<T>, Closeable
{
private final Iterator<T> iterator;
public CloseableIterator(Iterator<T> iterator)
{
this.iterator = iterator;
}
@Override
public boolean hasNext()
{
return iterator.hasNext();
}
@Override
public T next()
{
return iterator.next();
}
@Override
public void close()
{
closeFn.run();
}
}
}
}