[jira] [Comment Edited] (SOLR-8933) SolrDispatchFilter::consumeInput logs "Stream Closed" IOException

2016-04-08 Thread Uwe Schindler (JIRA)

[ 
https://issues.apache.org/jira/browse/SOLR-8933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15231700#comment-15231700
 ] 

Uwe Schindler edited comment on SOLR-8933 at 4/8/16 6:17 AM:
-

Hi,

bq. Doesn't matter that we don't use the extra APIs, the type signature of 
ServletRequest::getInputStream still demands a ServletInputStream so that is 
what we have to return when wrapping the request object. 

Yeah, I see you are wrapping the whole ServletRequest.

In any case, I skimmed through the patch: There is one problem. You don't close 
ContentStream's InputStream anymore. But there are cases in Solr, where the 
ContentStream does not come from a Request, but from somewhere else: It could 
also be the "virtual" stream from the URL param (OK this one does not need to 
be closed), but it could also be a remote stream e.g,, if you pass a stream.url 
parameter to the servlet. In that case it is never closed.

This would then be a bug!!! I'd suggest to do the following: Always wrap with 
CloseShield in DispatchFilter and ignore the close without logging anything 
(for safety with non-Jetty or later Jetty versions) and revert all other 
changes in Solr. To me it is unnatural to not close a stream after you have 
consumed it. I'd also change the consumers to try-with-resources.

The ContentStream tests are using remote streams (actually a file:// URL for 
testing). So on Windows, tests should fail with a resource leak there.


was (Author: thetaphi):
Hi,

bq. Doesn't matter that we don't use the extra APIs, the type signature of 
ServletRequest::getInputStream still demands a ServletInputStream so that is 
what we have to return when wrapping the request object. 

Yeah, I see you are wrapping the whole ServletRequest.

In any case, I skimmed through the patch: There is one problem. You don't close 
ContentStream's InputStream anymore. But there are cases in Solr, where the 
ContentStream does not come from a Request, but from somewhere else: It could 
also be the "virtual" stream from the URL param (OK this one does not need to 
be closed), but it could also be a remote stream e.g,, if you pass a stream.url 
parameter to the servlet. In that case it is never closed.

This would then be a bug!!! I'd suggest to do the following: Always wrap with 
CloseShield and ignore the close (for safety with non-Jetty or later Jetty 
versions) and revert all changes in Solr. To me it is unnatural to not close a 
stream after you have consumed it.

The ContentStream tests are using remote streams (actually a file:// URL for 
testing). So on Windows, tests should fail with a resource leak there.

> SolrDispatchFilter::consumeInput logs "Stream Closed" IOException
> -
>
> Key: SOLR-8933
> URL: https://issues.apache.org/jira/browse/SOLR-8933
> Project: Solr
>  Issue Type: Bug
>Affects Versions: 4.10.3
>Reporter: Mike Drob
>Assignee: Mark Miller
> Attachments: SOLR-8933.patch, SOLR-8933.patch, SOLR-8933.patch, 
> SOLR-8933.patch
>
>
> After SOLR-8453 we started seeing some IOExceptions coming out of 
> SolrDispatchFilter with "Stream Closed" messages.
> It looks like we are indeed closing the request stream in several places when 
> we really need to be letting the web container handle their life cycle. I've 
> got a preliminary patch ready and am working on testing it to make sure there 
> are no regressions.
> A very strange piece of this is that I have been entirely unable to reproduce 
> it on a unit test, but have seen it on cluster deployment quite consistently.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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



[jira] [Comment Edited] (SOLR-8933) SolrDispatchFilter::consumeInput logs "Stream Closed" IOException

2016-04-07 Thread Uwe Schindler (JIRA)

[ 
https://issues.apache.org/jira/browse/SOLR-8933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15231109#comment-15231109
 ] 

Uwe Schindler edited comment on SOLR-8933 at 4/7/16 9:32 PM:
-

bq. Which IOUtils? I didn't see this in o.a.lucene.util or o.a.solr.common.util.

Sorry I was thinking of commons-io instead of IOUtils. Was a typo!

bq. CloseShieldServletInputStream inspired by the Apache commons-io 
CloseShieldInputStream which was almost good enough to use. 

I would just use the CloseShieldInputStream. The additional APIs provided by 
ServletInputStream or ServletOutputStream are not used by Solr. Actually we put 
some of them to the forbidden-apis list (because of stupidity and ISO-8859-1 
defaults from earlier days). See lucene/tools/forbiddenApis/servlet-api.txt

I looked at this. There is no penalty by the wrapping - so you can also do it 
by default (with CloseShieldInputStream, no specialization). This is just 
optimized away... A method that just delegates to another methods is *always* 
removed by the VM.


was (Author: thetaphi):
bq. Which IOUtils? I didn't see this in o.a.lucene.util or o.a.solr.common.util.

Sorry I meat commons-io instead of IOUtils. Was a typo!

bq. CloseShieldServletInputStream inspired by the Apache commons-io 
CloseShieldInputStream which was almost good enough to use. 

I would just use the CloseShieldInputStream. The additional APIs provided by 
ServletInputStream or ServletOutputStream are not used by Solr. Actually we put 
some of them to the forbidden-apis list (because of stupidity and ISO-8859-1 
defaults from earlier days). See lucene/tools/forbiddenApis/servlet-api.txt

I looked at this. There is no penalty by the wrapping - so you can also do it 
by default (with CloseShieldInputStream, no specialization). This is just 
optimized away... A method that just delegates to another methods is *always* 
removed by the VM.

> SolrDispatchFilter::consumeInput logs "Stream Closed" IOException
> -
>
> Key: SOLR-8933
> URL: https://issues.apache.org/jira/browse/SOLR-8933
> Project: Solr
>  Issue Type: Bug
>Affects Versions: 4.10.3
>Reporter: Mike Drob
>Assignee: Mark Miller
> Attachments: SOLR-8933.patch, SOLR-8933.patch, SOLR-8933.patch, 
> SOLR-8933.patch
>
>
> After SOLR-8453 we started seeing some IOExceptions coming out of 
> SolrDispatchFilter with "Stream Closed" messages.
> It looks like we are indeed closing the request stream in several places when 
> we really need to be letting the web container handle their life cycle. I've 
> got a preliminary patch ready and am working on testing it to make sure there 
> are no regressions.
> A very strange piece of this is that I have been entirely unable to reproduce 
> it on a unit test, but have seen it on cluster deployment quite consistently.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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