You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sc...@apache.org on 2014/03/21 14:46:53 UTC
svn commit: r1579941 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: schultz
Date: Fri Mar 21 13:46:53 2014
New Revision: 1579941
URL: http://svn.apache.org/r1579941
Log:
Updated votes.
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1579941&r1=1579940&r2=1579941&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 13:46:53 2014
@@ -36,45 +36,22 @@ PATCHES PROPOSED TO BACKPORT:
+1: markt, kkolinko
-1: schultz: The idea of the patch is fine: I'm actually +1.
I have some small nits:
- 1. DocumentBuilderFactory is not thread-safe, and shouldn't
- be shared. 2. Two instances of swallowing IOException
+ 2. Two instances of swallowing IOException
when closing File streams. We should at least log a warning.
- It looks like there is an opportinity to use StringBuilder
- instead of StringBuffer, there, too, if you want.
+ The case of InputStream vs OutputStream is not relevant:
+ a stream left open should be logged. Honestly, it will
+ pretty much never happen, but that's no excuse not to log
+ a potential problem.
kkolinko:
- Re 1.:
- The newDocumentBuilder() method is thread safe.
-
- See JSR 206 (Final Release = JAXP 1.3)
- https://jcp.org/en/jsr/detail?id=206
- Ch.3 -> Thread Safety
- [quote]
- Implementations of the SAXParser, DocumentBuilder, Transformer, Validator and Validat
- orHandler abstract classes are not expected to be thread safe by this specification. (...)
-
- Configuration of a SAXParserFactory, DocumentBuilderFactory TransformerFactory or
- SchemaFactory is also not expected to be thread safe. (...)
-
- It is expected that the newSAXParser method of a SAXParserFactory implementation, the newDocument
- Builder method of a DocumentBuilderFactory and the newTransformer method of a Transformer
- Factory will be thread safe without side effects. (...)
-
- Note that Schema is thread safe.
- [/quote]
-
Re 2.:
Those are input streams that are read, not written. Nothing
should really happen when those are closed.
-
- Re StringBuilder:
- I think it is not of much concern. But if there is any interest,
- I am proposing a patch below. It is a 4 years old one.
-1:
* Use StringBuilder in DefaultServlet
Apply only DefaultServlet.java part of the following patch:
https://people.apache.org/~kkolinko/patches/StringBuilder/2009-11-02_StringBuilder_o_a_c_servlets.patch
- +1: kkolinko
+ +1: kkolinko, schultz
-1:
* Fix possible overflow when parsing long values from a byte array.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1579941 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,
On 3/21/14, 10:44 AM, Konstantin Kolinko wrote:
> 2014-03-21 17:46 GMT+04:00 <sc...@apache.org>:
>> Author: schultz
>> Date: Fri Mar 21 13:46:53 2014
>> New Revision: 1579941
>>
>> URL: http://svn.apache.org/r1579941
>> Log:
>> Updated votes.
>>
>> Modified:
>> tomcat/tc6.0.x/trunk/STATUS.txt
>>
>> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
>> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1579941&r1=1579940&r2=1579941&view=diff
>> ==============================================================================
>> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
>> +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 13:46:53 2014
>> @@ -36,45 +36,22 @@ PATCHES PROPOSED TO BACKPORT:
>> +1: markt, kkolinko
>> -1: schultz: The idea of the patch is fine: I'm actually +1.
>> I have some small nits:
>> - 1. DocumentBuilderFactory is not thread-safe, and shouldn't
>> - be shared. 2. Two instances of swallowing IOException
>> + 2. Two instances of swallowing IOException
>> when closing File streams. We should at least log a warning.
>> - It looks like there is an opportinity to use StringBuilder
>> - instead of StringBuffer, there, too, if you want.
>> + The case of InputStream vs OutputStream is not relevant:
>> + a stream left open should be logged. Honestly, it will
>> + pretty much never happen, but that's no excuse not to log
>> + a potential problem.
>
>
> 1) Is your vote still -1,
> or -0, or +0?
Still -1, just like STATUS.txt still says.
> 2) If inputStream.close() fails it does not mean that the stream is left open.
> Also no data is lost (unlike outputStream).
It doesn't mean that the stream is definitely left open. But the stream
*could* be left open.
> Anyway it cannot be a warning. It can be a debug message at best.
Why not a WARNING?
-chris
Re: svn commit: r1579941 - /tomcat/tc6.0.x/trunk/STATUS.txt
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-03-21 17:46 GMT+04:00 <sc...@apache.org>:
> Author: schultz
> Date: Fri Mar 21 13:46:53 2014
> New Revision: 1579941
>
> URL: http://svn.apache.org/r1579941
> Log:
> Updated votes.
>
> Modified:
> tomcat/tc6.0.x/trunk/STATUS.txt
>
> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1579941&r1=1579940&r2=1579941&view=diff
> ==============================================================================
> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
> +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 13:46:53 2014
> @@ -36,45 +36,22 @@ PATCHES PROPOSED TO BACKPORT:
> +1: markt, kkolinko
> -1: schultz: The idea of the patch is fine: I'm actually +1.
> I have some small nits:
> - 1. DocumentBuilderFactory is not thread-safe, and shouldn't
> - be shared. 2. Two instances of swallowing IOException
> + 2. Two instances of swallowing IOException
> when closing File streams. We should at least log a warning.
> - It looks like there is an opportinity to use StringBuilder
> - instead of StringBuffer, there, too, if you want.
> + The case of InputStream vs OutputStream is not relevant:
> + a stream left open should be logged. Honestly, it will
> + pretty much never happen, but that's no excuse not to log
> + a potential problem.
1) Is your vote still -1,
or -0, or +0?
2) If inputStream.close() fails it does not mean that the stream is left open.
Also no data is lost (unlike outputStream).
Anyway it cannot be a warning. It can be a debug message at best.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org