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 01:01:31 UTC
cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/servlets CGIServlet.java
amyroh 2002/08/26 16:01:31
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.2 +85 -51 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/servlets/CGIServlet.java
Index: CGIServlet.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/servlets/CGIServlet.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- CGIServlet.java 18 Jul 2002 16:47:39 -0000 1.1
+++ CGIServlet.java 26 Aug 2002 23:01:31 -0000 1.2
@@ -686,6 +686,7 @@
/** For future testing use only; does nothing right now */
public static void main(String[] args) {
System.out.println("$Header$");
+
}
@@ -1507,9 +1508,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 +1664,6 @@
* bugParade/bugs/4223650.html
*/
- boolean isRunning = true;
commandsStdOut = new BufferedReader
(new InputStreamReader(proc.getInputStream()));
commandsStdErr = new BufferedReader
@@ -1680,63 +1680,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>