You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2015/09/15 20:35:44 UTC
[Bug 58411] New: CoyoteWriter throws StringIndexOutOfBoundsException
when concurrently printing lines
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
Bug ID: 58411
Summary: CoyoteWriter throws StringIndexOutOfBoundsException
when concurrently printing lines
Product: Tomcat 8
Version: 8.0.26
Hardware: All
OS: All
Status: NEW
Severity: major
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: sdavids@gmail.com
I am currently opening multiple InputStreams and attempting to use println send
data through the Servlet's PrintWriter, unfortunately this sends back a
StringIndexOutOfBoundsException: String index out of range: -49
at java.lang.String.getChars(String.java:812)
at org.apache.tomcat.util.buf.CharChunk.append(CharChunk.java:333)
at org.apache.catalina.connector.OuputBuffer.write(OuputBuffer.java:533)
at org.apache.catalina.connector.CoyoteWriter.write(CoyoteWriter.java:170)
at org.apache.catalina.connector.CoyoteWriter.write(CoyoteWriter.java:180)
at org.apache.catalina.connector.CoyoteWriter.print(CoyoteWriter.java:238)
at org.apache.catalina.connector.CoyoteWriter.println(CoyoteWriter.java:305)
Using PrintWriter's printf or format methods work properly since the
CoyoteWriter doesn't override those methods and the appropriate synchronized
blocks are enforced, unlike the print overrides which specifically removed the
synchronized blocks. I would suggest removing the unnecessary overrides, it
looks like the PrintWriter handles everything the CoyoteWriter is doing. I
checked the history on the 8.x branch for the file and there isn't a reason why
the class was ever needed in the first place.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #13 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Remy Maucherat from comment #11)
> The thread safety issue actually came up fairly often in the past. And this
> doesn't make sense, why would it be partially synced ?
>
> The specification is perfectly explicit about it (2.3.3.4).
>
> "Other than the startAsync and complete methods, implementations of the
> request
> and response objects are not guaranteed to be thread safe. This means that
> they
> should either only be used within the scope of the request handling thread
> or the
> application must ensure that access to the request and response objects are
> thread
> safe.
> If a thread created by the application uses the container-managed objects,
> such as the
> request or response object, those objects must be accessed only within the
> object’s
> life cycle as defined in sections Section 3.12, “Lifetime of the Request
> Object” on
> page 3-31and Section 5.7, “Lifetime of the Response Object” on page 5-50
> respectively. Be aware that other than the startAsync , and complete
> methods, the
> request and response objects are not thread safe. If those objects were
> accessed in the
> multiple threads, the access should be synchronized or be done through a
> wrapper
> to add the thread safety, for instance, synchronizing the call of the
> methods to access
> the request attribute, or using a local output stream for the response
> object within a
> thread."
>
> Anyway, if despite the last sentence, you want to fix this "issue", go ahead
> :)
I would argue that the spec here limits its scope to the "request and response
objects" themselves. The servlet spec doesn't cover java.io.Writer, and
therefore the servlet spec is silent on the thread safety of the PrintWriter
returned by response.getWriter.
It may be worth noting that ServletOutputStream and OutputStream seem to say
nothing about thread safety, implying that one can use response.getOutputStream
to avoid the costs of synchronization if necessary.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #9 from Remy Maucherat <re...@apache.org> ---
Servlet 3.1 still says that Servlet objects are not thread safe. NIO2 does not
allow concurrency either, so there's no reason to sync this stream but not any
other one.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #15 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Remy Maucherat from comment #14)
> "or using a local output stream for the response object within a thread."
Re-reading that section several times, I think you're right that the servlet
spec mentions no requirements, here, but the Java API and associated Javadoc
still do. Specifically in the case of a Writer.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #11 from Remy Maucherat <re...@apache.org> ---
The thread safety issue actually came up fairly often in the past. And this
doesn't make sense, why would it be partially synced ?
The specification is perfectly explicit about it (2.3.3.4).
"Other than the startAsync and complete methods, implementations of the request
and response objects are not guaranteed to be thread safe. This means that they
should either only be used within the scope of the request handling thread or
the
application must ensure that access to the request and response objects are
thread
safe.
If a thread created by the application uses the container-managed objects, such
as the
request or response object, those objects must be accessed only within the
object’s
life cycle as defined in sections Section 3.12, “Lifetime of the Request
Object” on
page 3-31and Section 5.7, “Lifetime of the Response Object” on page 5-50
respectively. Be aware that other than the startAsync , and complete methods,
the
request and response objects are not thread safe. If those objects were
accessed in the
multiple threads, the access should be synchronized or be done through a
wrapper
to add the thread safety, for instance, synchronizing the call of the methods
to access
the request attribute, or using a local output stream for the response object
within a
thread."
Anyway, if despite the last sentence, you want to fix this "issue", go ahead :)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|INVALID |---
Status|RESOLVED |REOPENED
--- Comment #5 from Mark Thomas <ma...@apache.org> ---
However, the Javadoc for Writer references the protected lock object and has
done since Java 1.3 (as far back as I checked). The description is:
The object used to synchronize operations on this stream.
There is a strong implication in that description that all operations are
expected to synchronize on the lock object.
I'm re-opening this while we discuss it further to make sure it doesn't get
forgotten. It might still get resolved as INVALID again.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #6 from Remy Maucherat <re...@apache.org> ---
Really ? Those (legacy) Servlet objects have never been synced.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #4 from Steve Davids <sd...@gmail.com> ---
I have a distributed system that needs to collect data on each data node and
stream it back to the client in a timely manner, so I go ahead and open up
multiple concurrent InputStreams from each data node and stream it back to the
client in one OuputStream i.e. the Servlet's PrintWriter (CoyoteWriter for
Tomcat) in a rather simple callback method. I am not attempting to preserve
order as that would be a nightmarish task and isn't necessary for my use-case.
I did contemplate using a blocking queue but thought it was a premature
optimization before just trying to make my write method synchronized. The
synchronized method did the trick without a noticeable performance degradation,
so that's what I'm rolling with for the time being.
Just thought this was a bug and couldn't track down the original rational from
the file's history in GitHub. My initial thought was a speed optimization, but
I thought in recent versions of Java (1.4+) it isn't super costly.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #8 from Steve Davids <sd...@gmail.com> ---
PrintWriter doesn't say anything specifically about thread safety but the
Writer implies it with the constructor saying:
http://docs.oracle.com/javase/8/docs/api/java/io/Writer.html#Writer--
> Creates a new character-stream writer whose critical sections will synchronize on the writer itself.
Though, it is a bit ambiguous depending on what people consider "critical
sections".
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #2 from sdavids@gmail.com ---
Yes, that's what I did end up doing but it seems odd that Tomcat would
specifically remove the appropriate synchronization provided by the
PrintWriter, so my code that was working on Jetty doesn't work in Tomcat both
programming to the Servlet spec - one would think it would be a transparent
change. Is it even documented that Tomcat doesn't handle concurrent writes? Was
there a reason for adding this seemingly arbitrary restriction?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #14 from Remy Maucherat <re...@apache.org> ---
"or using a local output stream for the response object within a thread."
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
Status|REOPENED |RESOLVED
--- Comment #16 from Mark Thomas <ma...@apache.org> ---
The performance impact of adding the syncs is small but measurable (around 0.4%
based on my not very scientific testing) using a servlet that made 50x 1k
writes.
I've opened an issue with the EG to clarify the expected behaviour:
https://java.net/jira/browse/SERVLET_SPEC-140
I'm going to close this as INVALID for now. It can always be re-opened if the
clarification from the EG is that it should be thread safe.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
It's worth pointing-out that nowhere in java.io.PrintWriter does it say
anything about being thread-safe, nor does its superclass, java.io.Writer.
Also, none of the methods are synchronized.
Internally to these methods, there are synchronized blocks but that seems to be
an implementation detail.
If the CoyoteWriter is not thread-safe, it should probably documented as such,
but it's completely reasonable to leave it without synchronization: adding the
overhead of synchronization to every write() would be unreasonable for what is
usually a rare use-case.
You are already having to manage your threads to ensure that you write things
to the response in the right order (right?) so you may as well use that same
mechanism to provide thread-safety to your use of CoyoteWriter.
A better method of communication would be to queue your writes (using a
thread-safe queue) and have a single thread write to the response output
stream. Right now, it sounds like the wild west in your code.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #12 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Steve Davids from comment #8)
> PrintWriter doesn't say anything specifically about thread safety but the
> Writer implies it with the constructor saying:
>
> http://docs.oracle.com/javase/8/docs/api/java/io/Writer.html#Writer--
> > Creates a new character-stream writer whose critical sections will synchronize on the writer itself.
>
> Though, it is a bit ambiguous depending on what people consider "critical
> sections".
Sorry I missed that in the Writer javadoc. To me, this strongly implies that
all Writers are thread-safe.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
Remy Maucherat <re...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
Status|NEW |RESOLVED
--- Comment #1 from Remy Maucherat <re...@apache.org> ---
Concurrent writes (or reads ...) are not allowed, add synchronization.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #10 from Mark Thomas <ma...@apache.org> ---
The spec talks a lot about the request and response not being thread-safe. It
implies objects obtained from them are also not thread-safe but it isn't
explicit. You end up in a position where it is the implication of the Writer
Javadoc vs the implication of the Servlet spec. It isn't as clear cut as I
would like.
That this is the first time I can recall that this has come up it doesn't look
to be an issue for most users.
If the performance impact was neglibable then I'd be tempted to add the syncs.
The other option is ask the Servlet EG for a view.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 58411] CoyoteWriter throws StringIndexOutOfBoundsException when
concurrently printing lines
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58411
--- Comment #7 from Mark Thomas <ma...@apache.org> ---
I was all for closing it as INVALID as well then I went I dug through the
Javadoc. Is there anything you can recall in the Servlet spec, javadoc or the
EG discussions that points to the PrintWriter being thread-safe or not. I had a
look but couldn't find anything.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org