RE: DefaultServlet and getOutputStream() / getWriter()
Hi,
Additionally, every change I suggested (on tomcat-user) was definitly
not
changing any behaviour, but maybe improving performance.
The rewritten while{} patch you suggested definitely changed behavior
significantly, as I and others pointed out ;)
Returns a ServletOutputStream suitable for writing binary data in the
response. The servlet container does not encode the binary data.
java.lang.IllegalStateException - if the getWriter method has been
called
on
this response
java.io.IOException - if an input or output exception occurred
If I believe that javadocs, THERE IS NO REASON to do what the code
does,
sind getWriter is never called before getOutputStream, so there will
never
be the IllegalStateException and half of the code is obsolete.
When you're looking at the code, keep in mind that Tomcat's
DefaultServlet (like virtually every other Tomcat component) can be
extended or wrapped. Such wrappers or extenders could call getWriter
first.
In addition, since an exception CAN be thrown as the JavaDoc says, it's
only good practice to catch it: if the exception isn't thrown the
performance penalty on any modern JVM is virtually zero.
It also behooves us to be safe and careful in our code and design,
because we can never know all the possible uses of Tomcat. With Filters
and Wrappers, the request and response can be in various states
practically all along the processing pipeline, and so optimizations like
you suggest are risky at best.
I'm happy you're looking at the code. If I were in your position (and I
definitely was, although that seems long ago now ;)), I would look
instead at Bugzilla, take a bug, and try to fix it. The reason that's
better than just looking for optimizations in random places is that you
can test your work. You still gain familiarity with the Tomcat code, as
well as familiarity with the bug fixing / submission process, the CVS
environments, Tomcat's build, etc, all of which are necessary if you're
going to be submitting patches.
Thanks,
Yoav
This e-mail, including any attachments, is a confidential business communication, and
may contain information that is confidential, proprietary and/or privileged. This
e-mail is intended only for the individual(s) to whom it is addressed, and may not be
saved, copied, printed, disclosed or used by anyone else. If you are not the(an)
intended recipient, please immediately delete this e-mail from your computer system
and notify the sender. Thank you.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
RE: DefaultServlet and getOutputStream() / getWriter()
Hi,
I didn't look at the DefaultServlet code at all, there's no need. The post I was
referring to was one from you that said
Try { while { ... } } catch { ... }
Where either ... can throw an exception is the same as
While { ... try { ... } catch { ... } }
And obviously the two are not equal, because the second construct would keep working
the while loop and the first one would abort on the first error. That's a critical
difference. Now you're quoting specific DefaultServlet code, and that's fine, I don't
care to look at it now because it obviously works and I have more important things to
do. But your original message had just an abstract while/try/catch comparison
question, nothing specific to DefaultServlet, and that original message was wrong, and
that's the one I was responding to.
Yoav Shapira http://www.yoavshapira.com
-Original Message-
From: Steffen Heil [mailto:[EMAIL PROTECTED]
Sent: Tuesday, October 12, 2004 1:43 PM
To: 'Tomcat Developers List'
Subject: AW: DefaultServlet and getOutputStream() / getWriter()
Hi
The rewritten while{} patch you suggested definitely changed behavior
significantly, as I and others pointed out ;)
Ähm, no.
Sorry to say that, but I think, you didn't review the code for that
statement:
One example taken from DefaultServlet.java, lines 2030 to 2054:
IOException exception = null;
long bytesToRead = end - start + 1;
char buffer[] = new char[input];
int len = buffer.length;
while ( (bytesToRead 0) (len = buffer.length)) {
try {
len = reader.read(buffer);
if (bytesToRead = len) {
writer.write(buffer, 0, len);
bytesToRead -= len;
} else {
writer.write(buffer, 0, (int) bytesToRead);
bytesToRead = 0;
}
} catch (IOException e) {
exception = e;
len = -1;
}
if (len buffer.length)
break;
}
return exception;
THIS IS EQUAL TO:
IOException exception = null;
long bytesToRead = end - start + 1;
char buffer[] = new char[input];
int len = buffer.length;
try {
while ( (bytesToRead 0) (len = buffer.length)) {
len = reader.read(buffer);
if (bytesToRead = len) {
writer.write(buffer, 0, len);
bytesToRead -= len;
} else {
writer.write(buffer, 0, (int) bytesToRead);
bytesToRead = 0;
}
if (len buffer.length)
break;
}
} catch (IOException e) {
exception = e;
len = -1;
}
return exception;
OR EVEN:
long bytesToRead = end - start + 1;
char buffer[] = new char[input];
int len = buffer.length;
try {
while ( (bytesToRead 0) (len = buffer.length)) {
len = reader.read(buffer);
if (bytesToRead = len) {
writer.write(buffer, 0, len);
bytesToRead -= len;
} else {
writer.write(buffer, 0, (int) bytesToRead);
bytesToRead = 0;
}
}
return null;
} catch (IOException e) {
return e;
}
I am very sure about this.
And I also do NOT understand, why the exception is reported as result and
not really thrown.
The caller always uses:
IOException exception = null;
while ( (exception == null) (ranges.hasMoreElements()) ) {
exception = copyRange(istream, ostream, currentRange.start,
currentRange.end);
...
}
ostream.println();
ostream.print(-- + mimeSeparation + --);
// Rethrow any exception that has occurred
if (exception != null)
throw exception;
Whereas it would absolutely make more sense to me NOT to catch the
Exception
but rather use:
try {
while ( ranges.hasMoreElements() ) {
copyRange(istream, ostream, currentRange.start,
currentRange.end);
...
}
} finally {
// if nessesary, put code to ensure istream is closed here.
ostream.println();
ostream.print(-- + mimeSeparation + --);
}
This is what try-finally is all about, isn't it?
I agree, that I am new to this and I might be wrong, but this leads me back
right to where I started. Whom to ask to understand the existing code?
When you're looking at the code, keep in mind that Tomcat's
DefaultServlet
(like virtually every other Tomcat component) can be extended or wrapped.
Such wrappers or extenders could
Re: DefaultServlet and getOutputStream() / getWriter()
Responder, The email address you have contacted is no longer active. Please use your companys user log-in to www.apparelmagic.com/support to contact the ApparelMagic Support Department. Thank you. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DefaultServlet and getOutputStream() / getWriter()
Steffen Heil wrote:
Hi
I am reviewing the source code of the DefaultServlet.
I do not understand the following:
// slightly modified from source of serveResource.
ServletOutputStream ostream = null;
PrintWriter writer = null;
try {
ostream = response.getOutputStream();
} catch (IllegalStateException e) {
if ( contentType == null || contentType.startsWith( text ) )
writer = response.getWriter();
else
throw e;
}
}
Why would getOutputStream() fail?
And if that fails, why would getWriter() succeed?
Text data is also binary data, right?
I would understand that the other way around...
That's definitely why I am not interested by code cleanups done by folks
who might not know all the small tricks: the risk of breaking stuff is
far greater than the gain.
For this particular case, you should look into the API javadocs.
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: Re: DefaultServlet and getOutputStream() / getWriter()
Responder, The email address you have contacted is no longer active. Please use your companys user log-in to www.apparelmagic.com/support to contact the ApparelMagic Support Department. Thank you. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
