You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by am...@apache.org on 2002/08/27 00:38:35 UTC
cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/servlets CGIServlet.java
amyroh 2002/08/26 15:38:35
Modified: catalina/src/share/org/apache/catalina/servlets
CGIServlet.java
Log:
CGIServlet had problems in the case of a CGI script which prints a large
amount of data to stdout; while the CGIServlet was waiting for the end of
stderr, the CGI script would fill up the stdout buffer and then wait for it to be
drained, causing a deadlock between the two processes.
This patch solves this problem by having a single loop which
reads stderr if it's ready, or stdout if ready. If neither handle has queued
data and the CGI script has exited, the servlet pauses a couple of times for
half a second to make sure that all output has been delivered, then it exits.
Fixes Bug 12041. Patch submitted by Dave Glowacki (dglo@ssec.wisc.edu)
Revision Changes Path
1.8 +84 -51 jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/servlets/CGIServlet.java
Index: CGIServlet.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/servlets/CGIServlet.java,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- CGIServlet.java 20 Sep 2001 23:48:13 -0000 1.7
+++ CGIServlet.java 26 Aug 2002 22:38:35 -0000 1.8
@@ -1507,9 +1507,9 @@
* </UL>
* </p>
*
- * For more information, see java.lang.Runtime#exec(String command,
+ * For more information, see java.lang.Runtime#exec(String command,
* String[] envp, File dir)
- *
+ *
* @exception IOException if problems during reading/writing occur
*
*/
@@ -1663,7 +1663,6 @@
* bugParade/bugs/4223650.html
*/
- boolean isRunning = true;
commandsStdOut = new BufferedReader
(new InputStreamReader(proc.getInputStream()));
commandsStdErr = new BufferedReader
@@ -1680,63 +1679,97 @@
//NOOP: no output will be written
}
+ boolean inHeader = true;
+ int pauseCount = 0;
+
+ boolean isRunning = true;
+
while (isRunning) {
- try {
- //read stderr first
- cBuf = new char[1024];
- while ((bufRead = commandsStdErr.read(cBuf)) != -1) {
+ if (commandsStdErr != null && commandsStdErr.ready()) {
+ // about to read something; reset pause count
+ pauseCount = 0;
+
+ bufRead = commandsStdErr.read(cBuf);
+ if (bufRead == -1) {
+ commandsStdErr.close();
+ commandsStdErr = null;
+ isRunning = (commandsStdOut != null);
+ } else {
if (servletContainerStdout != null) {
- servletContainerStdout.write(cBuf, 0, bufRead);
+ if (inHeader) {
+ servletContainerStdout.write(cBuf, 0, bufRead);
+ } else {
+ log("runCGI: Throwing away StdErr \"" +
+ new String(cBuf, 0, bufRead) + "\"");
+ }
}
}
-
- //set headers
- String line = null;
- while (((line = commandsStdOut.readLine()) != null)
- && !("".equals(line))) {
- if (debug >= 2) {
- log("runCGI: addHeader(\"" + line + "\")");
- }
- if (line.startsWith("HTTP")) {
- //TODO: should set status codes (NPH support)
- /*
- * response.setStatus(getStatusCode(line));
- */
+ } else if (commandsStdOut != null && commandsStdOut.ready()) {
+ // about to read something; reset pause count
+ pauseCount = 0;
+
+ if (inHeader) {
+ //set headers
+ String line = commandsStdOut.readLine();
+ if (line == null || "".equals(line)) {
+ inHeader = false;
} else {
- response.addHeader
- (line.substring(0, line.indexOf(":")).trim(),
- line.substring(line.indexOf(":") + 1).trim());
+ if (debug >= 2) {
+ log("runCGI: addHeader(\"" + line + "\")");
+ }
+ if (line.startsWith("HTTP")) {
+ //TODO: should set status codes (NPH support)
+ /*
+ * response.setStatus(getStatusCode(line));
+ */
+ } else if (line.indexOf(":") >= 0) {
+ response.addHeader
+ (line.substring(0, line.indexOf(":")).trim(),
+ line.substring(line.indexOf(":") + 1).trim());
+ } else {
+ log("runCGI: bogus header line \"" + line + "\"");
+ }
}
- }
- //write output
- cBuf = new char[1024];
- while ((bufRead = commandsStdOut.read(cBuf)) != -1) {
- if (servletContainerStdout != null) {
- if (debug >= 4) {
- log("runCGI: write(\"" + cBuf + "\")");
+ } else {
+ //write output
+ bufRead = commandsStdOut.read(cBuf);
+ if (bufRead == -1) {
+ commandsStdOut.close();
+ commandsStdOut = null;
+ isRunning = (commandsStdErr != null);
+ } else {
+ if (servletContainerStdout != null) {
+ if (debug >= 4) {
+ log("runCGI: write(\"" + new String(cBuf, 0, bufRead) + "\")");
+ }
+ servletContainerStdout.write(cBuf, 0, bufRead);
}
- servletContainerStdout.write(cBuf, 0, bufRead);
}
}
- if (servletContainerStdout != null) {
- servletContainerStdout.flush();
- }
-
- proc.exitValue(); // Throws exception if alive
-
- isRunning = false;
-
- } catch (IllegalThreadStateException e) {
+ } else {
try {
- Thread.currentThread().sleep(500);
- } catch (InterruptedException ignored) {
+ int exitVal = proc.exitValue();
+ pauseCount++;
+ if (pauseCount > 2) {
+ isRunning = false;
+ } else {
+ // pause for half a second
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException ie) {
+ }
+ }
+ } catch (IllegalThreadStateException ex) {
}
}
- } //replacement for Process.waitFor()
+ if (servletContainerStdout != null) {
+ servletContainerStdout.flush();
+ }
+ } //replacement for Process.waitFor()
}
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/servlets
CGIServlet.java
Posted by Amy Roh <am...@apache.org>.
Remy Maucherat wrote:
> amyroh@apache.org wrote:
>
>> amyroh 2002/08/26 15:38:35
>>
>> Modified: catalina/src/share/org/apache/catalina/servlets
>> CGIServlet.java
>> Log:
>> CGIServlet had problems in the case of a CGI script which prints a
>> large
>> amount of data to stdout; while the CGIServlet was waiting for the
>> end of
>> stderr, the CGI script would fill up the stdout buffer and then wait
>> for it to be
>> drained, causing a deadlock between the two processes.
>> This patch solves this problem by having a single loop which
>> reads stderr if it's ready, or stdout if ready. If neither handle
>> has queued
>> data and the CGI script has exited, the servlet pauses a couple of
>> times for
>> half a second to make sure that all output has been delivered, then
>> it exits.
>> Fixes Bug 12041. Patch submitted by Dave Glowacki
>> (dglo@ssec.wisc.edu)
>
>
> I saw that patch, and wasn't happy about it, as it introduces some
> randomness in the script execution. Maybe it works fine, though, but it
> did look suspicious. Did you test it before applying it ?
I just tested simple perl scripts in tester and it worked fine.
>
> Could a CGI servlet expert comment on it ?
Yeah, that would be appreciated if someone with more CGI experience can
comment and test their scripts. I'll revert it back if there're problems.
Thanks,
Amy
>
> Thanks,
> Remy
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: cvs commit: jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/servlets
CGIServlet.java
Posted by Remy Maucherat <re...@apache.org>.
amyroh@apache.org wrote:
> amyroh 2002/08/26 15:38:35
>
> Modified: catalina/src/share/org/apache/catalina/servlets
> CGIServlet.java
> Log:
> CGIServlet had problems in the case of a CGI script which prints a large
> amount of data to stdout; while the CGIServlet was waiting for the end of
> stderr, the CGI script would fill up the stdout buffer and then wait for it to be
> drained, causing a deadlock between the two processes.
>
> This patch solves this problem by having a single loop which
> reads stderr if it's ready, or stdout if ready. If neither handle has queued
> data and the CGI script has exited, the servlet pauses a couple of times for
> half a second to make sure that all output has been delivered, then it exits.
>
> Fixes Bug 12041. Patch submitted by Dave Glowacki (dglo@ssec.wisc.edu)
I saw that patch, and wasn't happy about it, as it introduces some
randomness in the script execution. Maybe it works fine, though, but it
did look suspicious. Did you test it before applying it ?
Could a CGI servlet expert comment on it ?
Thanks,
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>