This commit breaks TestCSVLoader on Windows. It looks like Solr no longer 
closes any content streams that are passed separately to the servlet stream:

   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestCSVLoader 
-Dtests.method=testLiteral -Dtests.seed=B40869AE03F63CBC -Dtests.locale=ms 
-Dtests.timezone=ECT -Dtests.asserts=true -Dtests.file.encoding=UTF-8
   [junit4] ERROR   0.06s | TestCSVLoader.testLiteral <<<
   [junit4]    > Throwable #1: java.nio.file.FileSystemException: C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\solr\build\solr-core\test\J0\temp\solr.handler.TestCSVLoader_B40869AE03F63CBC-001\TestCSVLoader-006\solr_tmp.csv:
 Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen 
Prozess verwendet wird.
   [junit4]    >        at 
__randomizedtesting.SeedInfo.seed([B40869AE03F63CBC:B2ACAF677D87833E]:0)
   [junit4]    >        at 
sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
   [junit4]    >        at 
sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
   [junit4]    >        at 
sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
   [junit4]    >        at 
sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
   [junit4]    >        at 
sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
   [junit4]    >        at java.nio.file.Files.delete(Files.java:1126)
   [junit4]    >        at 
org.apache.solr.handler.TestCSVLoader.tearDown(TestCSVLoader.java:64)
   [junit4]    >        at java.lang.Thread.run(Thread.java:748)

I am not sure behind the reasoning of the whole thing, but IMHO, the better way 
to handle this is:
- Wrap the ServletInput/ServletOutput streams with a CloseShieldXxxxStream and 
only pass this around. This allows that code anywhere in solr is free to close 
any streams correctly, but the ServletStreams are kept open by the shield.

The reason behind everything here is a bug in Jetty. Jetty should prevent 
closing the stream!

I will reopen this issue. We now have a file descriptor leak also on Linux/Mac!

Uwe

-----
Uwe Schindler
Achterdiek 19, D-28357 Bremen
http://www.thetaphi.de
eMail: u...@thetaphi.de

> -----Original Message-----
> From: markrmil...@apache.org <markrmil...@apache.org>
> Sent: Saturday, May 5, 2018 1:02 AM
> To: comm...@lucene.apache.org
> Subject: lucene-solr:master: SOLR-12290: Do not close any servlet streams
> and improve our servlet stream closing prevention code for users and devs.
> 
> Repository: lucene-solr
> Updated Branches:
>   refs/heads/master ad0ad2ec8 -> 296201055
> 
> 
> SOLR-12290: Do not close any servlet streams and improve our servlet
> stream closing prevention code for users and devs.
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
> Commit: http://git-wip-us.apache.org/repos/asf/lucene-
> solr/commit/29620105
> Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/29620105
> Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/29620105
> 
> Branch: refs/heads/master
> Commit: 296201055f24f01e1610f2fb87aba7fa90b9dda1
> Parents: ad0ad2e
> Author: Mark Miller <markrmil...@apache.org>
> Authored: Fri May 4 18:02:06 2018 -0500
> Committer: Mark Miller <markrmil...@apache.org>
> Committed: Fri May 4 18:02:06 2018 -0500
> 
> ----------------------------------------------------------------------
>  solr/CHANGES.txt                                |   3 +
>  .../org/apache/solr/handler/BlobHandler.java    |   6 +-
>  .../apache/solr/handler/ReplicationHandler.java |   6 +-
>  .../solr/handler/loader/CSVLoaderBase.java      |  75 +++++-----
>  .../solr/handler/loader/JavabinLoader.java      |  16 +--
>  .../org/apache/solr/servlet/HttpSolrCall.java   |   6 +-
>  .../apache/solr/servlet/LoadAdminUiServlet.java |  14 +-
>  .../solr/servlet/ServletInputStreamWrapper.java |   2 +-
>  .../servlet/ServletOutputStreamWrapper.java     |   2 +-
>  .../apache/solr/servlet/SolrDispatchFilter.java | 139 ++++++++++++-------
>  .../apache/solr/servlet/SolrRequestParsers.java |   6 +-
>  11 files changed, 147 insertions(+), 128 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/CHANGES.txt
> ----------------------------------------------------------------------
> diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
> index d4c2097..f74e2fd 100644
> --- a/solr/CHANGES.txt
> +++ b/solr/CHANGES.txt
> @@ -206,6 +206,9 @@ Bug Fixes
> 
>  * SOLR-12202: Fix errors in solr-exporter.cmd. (Minoru Osuka via koji)
> 
> +* SOLR-12290: Do not close any servlet streams and improve our servlet
> stream closing prevention code for users
> +  and devs. (Mark Miller)
> +
>  Optimizations
>  ----------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/BlobHandl
> er.java
> ----------------------------------------------------------------------
> diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> index 30301c0..a998657 100644
> --- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> +++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> @@ -17,7 +17,6 @@
>  package org.apache.solr.handler;
> 
>  import java.io.IOException;
> -import java.io.InputStream;
>  import java.io.OutputStream;
>  import java.lang.invoke.MethodHandles;
>  import java.math.BigInteger;
> @@ -109,9 +108,8 @@ public class BlobHandler extends
> RequestHandlerBase implements PluginInfoInitial
> 
>        for (ContentStream stream : req.getContentStreams()) {
>          ByteBuffer payload;
> -        try (InputStream is = stream.getStream()) {
> -          payload = SimplePostTool.inputStreamToByteArray(is, maxSize);
> -        }
> +        payload =
> SimplePostTool.inputStreamToByteArray(stream.getStream(), maxSize);
> +
>          MessageDigest m = MessageDigest.getInstance("MD5");
>          m.update(payload.array(), payload.position(), payload.limit());
>          String md5 = new BigInteger(1, m.digest()).toString(16);
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/Replication
> Handler.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> index 1707c80..6afb8a0 100644
> --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> @@ -56,6 +56,7 @@ import java.util.zip.Checksum;
>  import java.util.zip.DeflaterOutputStream;
> 
>  import org.apache.commons.io.IOUtils;
> +import org.apache.commons.io.output.CloseShieldOutputStream;
>  import org.apache.lucene.codecs.CodecUtil;
>  import org.apache.lucene.index.DirectoryReader;
>  import org.apache.lucene.index.IndexCommit;
> @@ -1515,6 +1516,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
>      }
> 
>      protected void createOutputStream(OutputStream out) {
> +      out = new CloseShieldOutputStream(out); // DeflaterOutputStream
> requires a close call, but don't close the request outputstream
>        if (Boolean.parseBoolean(compress)) {
>          fos = new FastOutputStream(new DeflaterOutputStream(out));
>        } else {
> @@ -1579,7 +1581,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
>            }
>            if (read != buf.length) {
>              writeNothingAndFlush();
> -            fos.close();
> +            fos.close(); // we close because DeflaterOutputStream requires a
> close call, but but the request outputstream is protected
>              break;
>            }
>            offset += read;
> @@ -1638,7 +1640,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
>              long bytesRead = channel.read(bb);
>              if (bytesRead <= 0) {
>                writeNothingAndFlush();
> -              fos.close();
> +              fos.close(); // we close because DeflaterOutputStream requires 
> a
> close call, but but the request outputstream is protected
>                break;
>              }
>              fos.writeInt((int) bytesRead);
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/CSV
> LoaderBase.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> index b503fa3..fd8935d 100644
> ---
> a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> +++
> b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> @@ -28,7 +28,6 @@ import org.apache.solr.update.*;
>  import org.apache.solr.update.processor.UpdateRequestProcessor;
>  import org.apache.solr.internal.csv.CSVStrategy;
>  import org.apache.solr.internal.csv.CSVParser;
> -import org.apache.commons.io.IOUtils;
> 
>  import java.util.regex.Pattern;
>  import java.util.List;
> @@ -315,54 +314,48 @@ abstract class CSVLoaderBase extends
> ContentStreamLoader {
> 
>    /** load the CSV input */
>    @Override
> -  public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor) throws
> IOException {
> +  public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor)
> +      throws IOException {
>      errHeader = "CSVLoader: input=" + stream.getSourceInfo();
> -    Reader reader = null;
> -    try {
> -      reader = stream.getReader();
> -      if (skipLines>0) {
> -        if (!(reader instanceof BufferedReader)) {
> -          reader = new BufferedReader(reader);
> -        }
> -        BufferedReader r = (BufferedReader)reader;
> -        for (int i=0; i<skipLines; i++) {
> -          r.readLine();
> -        }
> +    Reader reader = stream.getReader();
> +    if (skipLines > 0) {
> +      if (!(reader instanceof BufferedReader)) {
> +        reader = new BufferedReader(reader);
>        }
> -
> -      CSVParser parser = new CSVParser(reader, strategy);
> -
> -      // parse the fieldnames from the header of the file
> -      if (fieldnames==null) {
> -        fieldnames = parser.getLine();
> -        if (fieldnames==null) {
> -          throw new SolrException(
> SolrException.ErrorCode.BAD_REQUEST,"Expected fieldnames in CSV input");
> -        }
> -        prepareFields();
> +      BufferedReader r = (BufferedReader) reader;
> +      for (int i = 0; i < skipLines; i++) {
> +        r.readLine();
>        }
> +    }
> 
> -      // read the rest of the CSV file
> -      for(;;) {
> -        int line = parser.getLineNumber();  // for error reporting in MT mode
> -        String[] vals = null;
> -        try {
> -          vals = parser.getLine();
> -        } catch (IOException e) {
> -          //Catch the exception and rethrow it with more line information
> -         input_err("can't read line: " + line, null, line, e);
> -        }
> -        if (vals==null) break;
> +    CSVParser parser = new CSVParser(reader, strategy);
> 
> -        if (vals.length != fieldnames.length) {
> -          input_err("expected "+fieldnames.length+" values but got
> "+vals.length, vals, line);
> -        }
> +    // parse the fieldnames from the header of the file
> +    if (fieldnames == null) {
> +      fieldnames = parser.getLine();
> +      if (fieldnames == null) {
> +        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
> "Expected fieldnames in CSV input");
> +      }
> +      prepareFields();
> +    }
> 
> -        addDoc(line,vals);
> +    // read the rest of the CSV file
> +    for (;;) {
> +      int line = parser.getLineNumber(); // for error reporting in MT mode
> +      String[] vals = null;
> +      try {
> +        vals = parser.getLine();
> +      } catch (IOException e) {
> +        // Catch the exception and rethrow it with more line information
> +        input_err("can't read line: " + line, null, line, e);
>        }
> -    } finally{
> -      if (reader != null) {
> -        IOUtils.closeQuietly(reader);
> +      if (vals == null) break;
> +
> +      if (vals.length != fieldnames.length) {
> +        input_err("expected " + fieldnames.length + " values but got " +
> vals.length, vals, line);
>        }
> +
> +      addDoc(line, vals);
>      }
>    }
> 
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/Jav
> abinLoader.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> index f502a8e..aca3df4 100644
> --- a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> +++
> b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> @@ -48,20 +48,8 @@ import
> org.apache.solr.update.processor.UpdateRequestProcessor;
>  public class JavabinLoader extends ContentStreamLoader {
> 
>    @Override
> -  public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor) throws
> Exception {
> -    InputStream is = null;
> -    try {
> -      is = stream.getStream();
> -      parseAndLoadDocs(req, rsp, is, processor);
> -    } finally {
> -      if(is != null) {
> -        is.close();
> -      }
> -    }
> -  }
> -
> -  private void parseAndLoadDocs(final SolrQueryRequest req,
> SolrQueryResponse rsp, InputStream stream,
> -                                final UpdateRequestProcessor processor) 
> throws
> IOException {
> +  public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream cs, UpdateRequestProcessor processor) throws Exception {
> +    InputStream stream = cs.getStream();
>      UpdateRequest update = null;
>      JavaBinUpdateRequestCodec.StreamingUpdateHandler handler = new
> JavaBinUpdateRequestCodec.StreamingUpdateHandler() {
>        private AddUpdateCommand addCmd = null;
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.
> java
> ----------------------------------------------------------------------
> diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> index 50fa71c..5e4dd78 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> @@ -39,8 +39,6 @@ import java.util.Set;
>  import java.util.concurrent.TimeUnit;
> 
>  import org.apache.commons.io.IOUtils;
> -import org.apache.commons.io.input.CloseShieldInputStream;
> -import org.apache.commons.io.output.CloseShieldOutputStream;
>  import org.apache.commons.lang.StringUtils;
>  import org.apache.http.Header;
>  import org.apache.http.HeaderIterator;
> @@ -590,7 +588,7 @@ public class HttpSolrCall {
>        } else if (isPostOrPutRequest) {
>          HttpEntityEnclosingRequestBase entityRequest =
>              "POST".equals(req.getMethod()) ? new HttpPost(urlstr) : new
> HttpPut(urlstr);
> -        InputStream in = new CloseShieldInputStream(req.getInputStream()); //
> Prevent close of container streams
> +        InputStream in = req.getInputStream();
>          HttpEntity entity = new InputStreamEntity(in, 
> req.getContentLength());
>          entityRequest.setEntity(entity);
>          method = entityRequest;
> @@ -785,7 +783,7 @@ public class HttpSolrCall {
>        }
> 
>        if (Method.HEAD != reqMethod) {
> -        OutputStream out = new
> CloseShieldOutputStream(response.getOutputStream()); // Prevent close of
> container streams, see SOLR-8933
> +        OutputStream out = response.getOutputStream();
>          QueryResponseWriterUtil.writeQueryResponse(out, responseWriter,
> solrReq, solrRsp, ct);
>        }
>        //else http HEAD request, nothing to write out, waited this long just 
> to
> get ContentType
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/LoadAdmin
> UiServlet.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> index c496ce1..1aa1137 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> @@ -17,7 +17,6 @@
>  package org.apache.solr.servlet;
> 
>  import org.apache.commons.io.IOUtils;
> -import org.apache.commons.io.output.CloseShieldOutputStream;
>  import org.apache.commons.lang.StringEscapeUtils;
>  import org.apache.commons.lang.StringUtils;
>  import org.apache.solr.common.params.CommonParams;
> @@ -41,10 +40,12 @@ import java.nio.charset.StandardCharsets;
>  public final class LoadAdminUiServlet extends BaseSolrServlet {
> 
>    @Override
> -  public void doGet(HttpServletRequest request,
> -                    HttpServletResponse response)
> +  public void doGet(HttpServletRequest _request,
> +                    HttpServletResponse _response)
>        throws IOException {
> -
> +    HttpServletRequest request = SolrDispatchFilter.closeShield(_request,
> false);
> +    HttpServletResponse response =
> SolrDispatchFilter.closeShield(_response, false);
> +
>      response.addHeader("X-Frame-Options", "DENY"); // security: SOLR-7966
> - avoid clickjacking for admin interface
> 
>      // This attribute is set by the SolrDispatchFilter
> @@ -57,8 +58,8 @@ public final class LoadAdminUiServlet extends
> BaseSolrServlet {
>          response.setCharacterEncoding("UTF-8");
>          response.setContentType("text/html");
> 
> -        // Protect container owned streams from being closed by us, see SOLR-
> 8933
> -        out = new OutputStreamWriter(new
> CloseShieldOutputStream(response.getOutputStream()),
> StandardCharsets.UTF_8);
> +        // Don't close this! - see SOLR-8933
> +        out = new OutputStreamWriter(response.getOutputStream(),
> StandardCharsets.UTF_8);
> 
>          String html = IOUtils.toString(in, "UTF-8");
>          Package pack = SolrCore.class.getPackage();
> @@ -77,7 +78,6 @@ public final class LoadAdminUiServlet extends
> BaseSolrServlet {
>          out.write( StringUtils.replaceEach(html, search, replace) );
>        } finally {
>          IOUtils.closeQuietly(in);
> -        IOUtils.closeQuietly(out);
>        }
>      } else {
>        response.sendError(404);
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletInput
> StreamWrapper.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> index d229bf7..826dc59 100644
> ---
> a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> +++
> b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> @@ -32,7 +32,7 @@ import
> org.apache.solr.common.util.SuppressForbidden;
>   */
>  @SuppressForbidden(reason = "delegate methods")
>  public class ServletInputStreamWrapper extends ServletInputStream {
> -  final ServletInputStream stream;
> +  ServletInputStream stream;
> 
>    public ServletInputStreamWrapper(ServletInputStream stream) throws
> IOException {
>      this.stream = stream;
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletOutp
> utStreamWrapper.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.j
> ava
> b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.
> java
> index d12c3bd..1164146 100644
> ---
> a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.j
> ava
> +++
> b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.
> java
> @@ -32,7 +32,7 @@ import
> org.apache.solr.common.util.SuppressForbidden;
>   */
>  @SuppressForbidden(reason = "delegate methods")
>  public class ServletOutputStreamWrapper extends ServletOutputStream {
> -  final ServletOutputStream stream;
> +  ServletOutputStream stream;
> 
>    public ServletOutputStreamWrapper(ServletOutputStream stream) {
>      this.stream = stream;
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/SolrDispatch
> Filter.java
> ----------------------------------------------------------------------
> diff --git 
> a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> index fc0c28f..8f32a7d 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> @@ -37,20 +37,20 @@ import java.util.regex.Pattern;
> 
>  import javax.servlet.FilterChain;
>  import javax.servlet.FilterConfig;
> +import javax.servlet.ReadListener;
>  import javax.servlet.ServletException;
>  import javax.servlet.ServletInputStream;
>  import javax.servlet.ServletOutputStream;
>  import javax.servlet.ServletRequest;
>  import javax.servlet.ServletResponse;
>  import javax.servlet.UnavailableException;
> +import javax.servlet.WriteListener;
>  import javax.servlet.http.HttpServletRequest;
>  import javax.servlet.http.HttpServletRequestWrapper;
>  import javax.servlet.http.HttpServletResponse;
>  import javax.servlet.http.HttpServletResponseWrapper;
> 
>  import org.apache.commons.io.FileCleaningTracker;
> -import org.apache.commons.io.input.CloseShieldInputStream;
> -import org.apache.commons.io.output.CloseShieldOutputStream;
>  import org.apache.commons.lang.StringUtils;
>  import org.apache.http.client.HttpClient;
>  import org.apache.lucene.util.Version;
> @@ -98,8 +98,6 @@ public class SolrDispatchFilter extends BaseSolrFilter {
>    protected HttpClient httpClient;
>    private ArrayList<Pattern> excludePatterns;
> 
> -  // Effectively immutable
> -  private Boolean testMode = null;
>    private boolean isV2Enabled =
> !"true".equals(System.getProperty("disable.v2.api", "false"));
> 
>    private final String metricTag = Integer.toHexString(hashCode());
> @@ -119,19 +117,6 @@ public class SolrDispatchFilter extends BaseSolrFilter
> {
>    }
> 
>    public SolrDispatchFilter() {
> -    // turn on test mode when running tests
> -    assert testMode = true;
> -
> -    if (testMode == null) {
> -      testMode = false;
> -    } else {
> -      String tm =
> System.getProperty("solr.tests.doContainerStreamCloseAssert");
> -      if (tm != null) {
> -        testMode = Boolean.parseBoolean(tm);
> -      } else {
> -        testMode = true;
> -      }
> -    }
>    }
> 
>    public static final String PROPERTIES_ATTRIBUTE = "solr.properties";
> @@ -341,8 +326,8 @@ public class SolrDispatchFilter extends BaseSolrFilter {
> 
>    public void doFilter(ServletRequest _request, ServletResponse _response,
> FilterChain chain, boolean retry) throws IOException, ServletException {
>      if (!(_request instanceof HttpServletRequest)) return;
> -    HttpServletRequest request = (HttpServletRequest)_request;
> -    HttpServletResponse response = (HttpServletResponse)_response;
> +    HttpServletRequest request = closeShield((HttpServletRequest)_request,
> retry);
> +    HttpServletResponse response =
> closeShield((HttpServletResponse)_response, retry);
> 
>      try {
> 
> @@ -387,7 +372,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
>          }
>        }
> 
> -      HttpSolrCall call = getHttpSolrCall(closeShield(request, retry),
> closeShield(response, retry), retry);
> +      HttpSolrCall call = getHttpSolrCall(request, response, retry);
>        ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
>        try {
>          Action result = call.call();
> @@ -486,31 +471,82 @@ public class SolrDispatchFilter extends
> BaseSolrFilter {
>      return true;
>    }
> 
> +  public static class ClosedServletInputStream extends ServletInputStream {
> +
> +    public static final ClosedServletInputStream
> CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream();
> +
> +    @Override
> +    public int read() {
> +      return -1;
> +    }
> +
> +    @Override
> +    public boolean isFinished() {
> +      return false;
> +    }
> +
> +    @Override
> +    public boolean isReady() {
> +      return false;
> +    }
> +
> +    @Override
> +    public void setReadListener(ReadListener arg0) {}
> +  }
> +
> +  public static class ClosedServletOutputStream extends
> ServletOutputStream {
> +
> +    public static final ClosedServletOutputStream
> CLOSED_SERVLET_OUTPUT_STREAM = new ClosedServletOutputStream();
> +
> +    @Override
> +    public void write(final int b) throws IOException {
> +      throw new IOException("write(" + b + ") failed: stream is closed");
> +    }
> +
> +    @Override
> +    public void flush() throws IOException {
> +      throw new IOException("flush() failed: stream is closed");
> +    }
> +
> +    @Override
> +    public boolean isReady() {
> +      return false;
> +    }
> +
> +    @Override
> +    public void setWriteListener(WriteListener arg0) {
> +      throw new RuntimeException("setWriteListener() failed: stream is
> closed");
> +    }
> +  }
> +
>    /**
> -   * Wrap the request's input stream with a close shield, as if by a {@link
> CloseShieldInputStream}. If this is a
> +   * Wrap the request's input stream with a close shield. If this is a
>     * retry, we will assume that the stream has already been wrapped and do
> nothing.
>     *
> +   * Only the container should ever actually close the servlet output stream.
> +   *
>     * @param request The request to wrap.
>     * @param retry If this is an original request or a retry.
>     * @return A request object with an {@link InputStream} that will ignore
> calls to close.
>     */
> -  private HttpServletRequest closeShield(HttpServletRequest request,
> boolean retry) {
> -    if (testMode && !retry) {
> +  public static HttpServletRequest closeShield(HttpServletRequest request,
> boolean retry) {
> +    if (!retry) {
>        return new HttpServletRequestWrapper(request) {
> -        ServletInputStream stream;
> -
> +
>          @Override
>          public ServletInputStream getInputStream() throws IOException {
> -          // Lazy stream creation
> -          if (stream == null) {
> -            stream = new ServletInputStreamWrapper(super.getInputStream()) {
> -              @Override
> -              public void close() {
> -                assert false : "Attempted close of request input stream.";
> -              }
> -            };
> -          }
> -          return stream;
> +
> +          return new ServletInputStreamWrapper(super.getInputStream()) {
> +            @Override
> +            public void close() {
> +              // even though we skip closes, we let local tests know not to 
> close so
> that a full understanding can take
> +              // place
> +              assert
> Thread.currentThread().getStackTrace()[2].getClassName().matches(
> +                  "org\\.apache\\.(?:solr|lucene).*") ? false : true : 
> "Attempted
> close of request input stream - never do this, you will spoil connection reuse
> and possibly disrupt a client";
> +              this.stream =
> ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM;
> +            }
> +          };
> +
>          }
>        };
>      } else {
> @@ -519,31 +555,34 @@ public class SolrDispatchFilter extends
> BaseSolrFilter {
>    }
> 
>    /**
> -   * Wrap the response's output stream with a close shield, as if by a {@link
> CloseShieldOutputStream}. If this is a
> +   * Wrap the response's output stream with a close shield. If this is a
>     * retry, we will assume that the stream has already been wrapped and do
> nothing.
>     *
> +   * Only the container should ever actually close the servlet request 
> stream.
> +   *
>     * @param response The response to wrap.
>     * @param retry If this response corresponds to an original request or a
> retry.
>     * @return A response object with an {@link OutputStream} that will ignore
> calls to close.
>     */
> -  private HttpServletResponse closeShield(HttpServletResponse response,
> boolean retry) {
> -    if (testMode && !retry) {
> +  public static HttpServletResponse closeShield(HttpServletResponse
> response, boolean retry) {
> +    if (!retry) {
>        return new HttpServletResponseWrapper(response) {
> -        ServletOutputStream stream;
> -
> +
>          @Override
>          public ServletOutputStream getOutputStream() throws IOException {
> -          // Lazy stream creation
> -          if (stream == null) {
> -            stream = new
> ServletOutputStreamWrapper(super.getOutputStream()) {
> -              @Override
> -              public void close() {
> -                assert false : "Attempted close of response output stream.";
> -              }
> -            };
> -          }
> -          return stream;
> +
> +          return new ServletOutputStreamWrapper(super.getOutputStream()) {
> +            @Override
> +            public void close() {
> +              // even though we skip closes, we let local tests know not to 
> close so
> that a full understanding can take
> +              // place
> +              assert
> Thread.currentThread().getStackTrace()[2].getClassName().matches(
> +                  "org\\.apache\\.(?:solr|lucene).*") ? false : true : 
> "Attempted
> close of response output stream - never do this, you will spoil connection
> reuse and possibly disrupt a client";
> +              stream =
> ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM;
> +            }
> +          };
>          }
> +
>        };
>      } else {
>        return response;
> 
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/SolrRequest
> Parsers.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> index 60b6d2f..4bcb8d8 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> @@ -519,8 +519,7 @@ public class SolrRequestParsers
> 
>      @Override
>      public InputStream getStream() throws IOException {
> -      // Protect container owned streams from being closed by us, see SOLR-
> 8933
> -      return new CloseShieldInputStream(req.getInputStream());
> +      return req.getInputStream();
>      }
>    }
> 
> @@ -767,8 +766,7 @@ public class SolrRequestParsers
>          String userAgent = req.getHeader("User-Agent");
>          boolean isCurl = userAgent != null && userAgent.startsWith("curl/");
> 
> -        // Protect container owned streams from being closed by us, see SOLR-
> 8933
> -        FastInputStream input = FastInputStream.wrap( new
> CloseShieldInputStream(req.getInputStream()) );
> +        FastInputStream input = FastInputStream.wrap(req.getInputStream());
> 
>          if (isCurl) {
>            SolrParams params = autodetect(req, streams, input);


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to