DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=34801>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=34801
Summary: PATCH: CGIServlet does not kill CGI child process on
IOException, nor terminate child after a timeout
Product: Tomcat 5
Version: 5.5.9
Platform: Other
OS/Version: Windows XP
Status: NEW
Severity: normal
Priority: P2
Component: Catalina
AssignedTo: tomcat-dev@jakarta.apache.org
ReportedBy: chris_j_davey@hotmail.com
CGIServlet does not kill CGI child process on IOException, nor terminate child
after a timeout.
We had a problem where a CGI child process (in perl, on windows) remained
running for a long time. In some situations where the client disconnected, an
IOException was thrown in the servlet, but this did not result in a cleanup of
the CGI child process. This was a problem as our CGI program accessed a single
threaded resource over TCP, and could therefore lock the system up indefinitly.
===
--- CGIServlet.java Sun May 08 04:46:05 2005
+++
C:\Temp\hjakarta-tomcat-5.5.9-src.tar\jakarta-tomcat-5.5.9-src\jakarta-tomcat-catalina\catalina\src\share\org\apache\catalina\servlets\CGIServlet.java
Sat Mar 26 18:24:02 2005
@@ -267,8 +267,6 @@
private String parameterEncoding = System.getProperty("file.encoding",
"UTF-8");
- private long lScriptTimeoutMillis = 2000;
-
/** object used to ensure multiple threads don't try to expand same file */
static Object expandFileLock = new Object();
@@ -332,16 +330,10 @@
cgiExecutable = value;
}
-
value = getServletConfig().getInitParameter("parameterEncoding");
if (value != null) {
parameterEncoding = value;
}
- // Added by CJD
- value = getServletConfig().getInitParameter("scriptTimeout");
- if (value != null) {
- lScriptTimeoutMillis = Integer.valueOf(value).intValue();
- }
// Identify the internal container resources we need
context = config.getServletContext();
@@ -1440,7 +1432,7 @@
*/
protected class CGIRunner {
- private Process proc = null;
+
/** script/command to be executed */
private String command = null;
@@ -1656,6 +1648,7 @@
InputStream cgiOutput = null;
BufferedReader commandsStdErr = null;
BufferedOutputStream commandsStdIn = null;
+ Process proc = null;
int bufRead = -1;
//create query arguments
@@ -1731,200 +1724,138 @@
}
rt = Runtime.getRuntime();
- try {
+ proc = rt.exec(cmdAndArgs.toString(), hashToStringArray(env), wd);
- proc = rt.exec(cmdAndArgs.toString(), hashToStringArray(env), wd);
+ if(contentStream != null) {
+ commandsStdIn = new BufferedOutputStream(proc.getOutputStream());
+ commandsStdIn.write(contentStream.toByteArray());
+ commandsStdIn.flush();
+ commandsStdIn.close();
+ }
- if(contentStream != null) {
- commandsStdIn = new
BufferedOutputStream(proc.getOutputStream());
- commandsStdIn.write(contentStream.toByteArray());
- commandsStdIn.flush();
- commandsStdIn.close();
- }
+ /* we want to wait for the process to exit, Process.waitFor()
+ * is useless in our situation; see
+ * http://developer.java.sun.com/developer/
+ * bugParade/bugs/4223650.html
+ */
- /* we want to wait for the process to exit, Process.waitFor()
- * is useless in our situation; see
- * http://developer.java.sun.com/developer/
- * bugParade/bugs/4223650.html
- */
-
- boolean isRunning = true;
- commandsStdErr = new BufferedReader
- (new InputStreamReader(proc.getErrorStream()));
- BufferedWriter servletContainerStdout = null;
+ boolean isRunning = true;
+ commandsStdErr = new BufferedReader
+ (new InputStreamReader(proc.getErrorStream()));
+ BufferedWriter servletContainerStdout = null;
- try {
- if (response.getOutputStream() != null) {
- servletContainerStdout =
- new BufferedWriter(new OutputStreamWriter
- (response.getOutputStream()));
- }
- } catch (IOException ignored) {
- //NOOP: no output will be written
+ try {
+ if (response.getOutputStream() != null) {
+ servletContainerStdout =
+ new BufferedWriter(new OutputStreamWriter
+ (response.getOutputStream()));
}
- final BufferedReader stdErrRdr = commandsStdErr ;
+ } catch (IOException ignored) {
+ //NOOP: no output will be written
+ }
+ final BufferedReader stdErrRdr = commandsStdErr ;
- new Thread() {
- public void run () {
- sendToLog(stdErrRdr) ;
- } ;
- }.start() ;
-
-
- new Thread() {
- public void run () {
- deadProcWatcher(proc,lScriptTimeoutMillis) ;
- } ;
- }.start() ;
-
- InputStream cgiHeaderStream =
- new HTTPHeaderInputStream(proc.getInputStream());
- BufferedReader cgiHeaderReader =
- new BufferedReader(new InputStreamReader(cgiHeaderStream));
- boolean isBinaryContent = false;
+ new Thread() {
+ public void run () {
+ sendToLog(stdErrRdr) ;
+ } ;
+ }.start() ;
+
+ InputStream cgiHeaderStream =
+ new HTTPHeaderInputStream(proc.getInputStream());
+ BufferedReader cgiHeaderReader =
+ new BufferedReader(new InputStreamReader(cgiHeaderStream));
+ boolean isBinaryContent = false;
-
-
- while (isRunning) {
- try {
- //set headers
- String line = null;
- while (((line = cgiHeaderReader.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 (line.indexOf(":") >= 0) {
- String header =
- line.substring(0, line.indexOf(":")).trim();
- String value =
- line.substring(line.indexOf(":") + 1).trim();
- response.addHeader(header , value);
- if ((header.toLowerCase().equals("content-type"))
- && (!value.toLowerCase().startsWith("text")))
{
- isBinaryContent = true;
- }
- } else {
- log("runCGI: bad header line \"" + line + "\"");
+ while (isRunning) {
+ try {
+ //set headers
+ String line = null;
+ while (((line = cgiHeaderReader.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 (line.indexOf(":") >= 0) {
+ String header =
+ line.substring(0, line.indexOf(":")).trim();
+ String value =
+ line.substring(line.indexOf(":") + 1).trim();
+ response.addHeader(header , value);
+ if ((header.toLowerCase().equals("content-type"))
+ && (!value.toLowerCase().startsWith("text"))) {
+ isBinaryContent = true;
}
+ } else {
+ log("runCGI: bad header line \"" + line + "\"");
}
+ }
- //write output
- if (isBinaryContent) {
- byte[] bBuf = new byte[2048];
- OutputStream out = response.getOutputStream();
- cgiOutput = proc.getInputStream();
- while ((bufRead = cgiOutput.read(bBuf)) != -1) {
- if (debug >= 4) {
- log("runCGI: output " + bufRead +
- " bytes of binary data");
- }
- out.write(bBuf, 0, bufRead);
+ //write output
+ if (isBinaryContent) {
+ byte[] bBuf = new byte[2048];
+ OutputStream out = response.getOutputStream();
+ cgiOutput = proc.getInputStream();
+ while ((bufRead = cgiOutput.read(bBuf)) != -1) {
+ if (debug >= 4) {
+ log("runCGI: output " + bufRead +
+ " bytes of binary data");
}
- } else {
- commandsStdOut = new BufferedReader
- (new InputStreamReader(proc.getInputStream()));
+ out.write(bBuf, 0, bufRead);
+ }
+ } else {
+ commandsStdOut = new BufferedReader
+ (new InputStreamReader(proc.getInputStream()));
- char[] cBuf = new char[1024];
- try {
- while ((bufRead = commandsStdOut.read(cBuf)) !=
-1) {
- if (servletContainerStdout != null) {
- if (debug >= 4) {
- log("runCGI: write(\"" +
+ char[] cBuf = new char[1024];
+ try {
+ while ((bufRead = commandsStdOut.read(cBuf)) != -1) {
+ if (servletContainerStdout != null) {
+ if (debug >= 4) {
+ log("runCGI: write(\"" +
new String(cBuf, 0, bufRead) +
"\")");
- }
- servletContainerStdout.write(cBuf, 0,
bufRead);
}
- }
- } finally {
- // Attempt to consume any leftover byte if
something bad happens,
- // such as a socket disconnect on the servlet
side; otherwise, the
- // external process could hang
- if (bufRead != -1) {
- while ((bufRead =
commandsStdOut.read(cBuf)) != -1) {}
+ servletContainerStdout.write(cBuf, 0, bufRead);
}
}
-
- if (servletContainerStdout != null) {
- servletContainerStdout.flush();
+ } finally {
+ // Attempt to consume any leftover byte if
something bad happens,
+ // such as a socket disconnect on the servlet side;
otherwise, the
+ // external process could hang
+ if (bufRead != -1) {
+ while ((bufRead = commandsStdOut.read(cBuf)) !=
-1) {}
}
}
-
- proc.exitValue(); // Throws exception if alive
-
- isRunning = false;
-
- } catch (IllegalThreadStateException e) {
- try {
- Thread.sleep(500);
- } catch (InterruptedException ignored) {
+
+ if (servletContainerStdout != null) {
+ servletContainerStdout.flush();
}
}
-
- } //replacement for Process.waitFor()
- // Close the output stream used
- if (isBinaryContent) {
- cgiOutput.close();
- } else {
- commandsStdOut.close();
- }
- }
- catch (IOException e){
- log ("Caught exception " + e);
- System.out.println("Caught IOException - printing dump 1");
- e.printStackTrace();
- if (proc != null){
- proc.destroy();
- proc = null;
- }
- throw new IOException (e.toString());
- }
- finally{
- log ("Running finally block");
- if (proc != null){
- proc.destroy();
- proc = null;
- }
- }
- }
- private void deadProcWatcher(Process myproc,long lWait){
- long processRunDeadline = System.currentTimeMillis() +lWait;
+ proc.exitValue(); // Throws exception if alive
+
+ isRunning = false;
- while(System.currentTimeMillis() < processRunDeadline){
- log("deadProcWatcher: cgi run: " +
- processRunDeadline + " " +
- System.currentTimeMillis() +
- " " + myproc);
- try{
- myproc.exitValue(); // Throws exception if alive
- log("deadProcWatcher: exiting normally");
- return; // child has exited, we can exit
-
} catch (IllegalThreadStateException e) {
try {
Thread.sleep(500);
} catch (InterruptedException ignored) {
}
}
+ } //replacement for Process.waitFor()
+ // Close the output stream used
+ if (isBinaryContent) {
+ cgiOutput.close();
+ } else {
+ commandsStdOut.close();
}
- if (System.currentTimeMillis() > processRunDeadline){
- log("Killing process due to timeout" +
- processRunDeadline + " " +
- System.currentTimeMillis() +
- " " + myproc);
- myproc.destroy();
- }
-
}
-
-
private void sendToLog(BufferedReader rdr) {
String line = null;
int lineCount = 0 ;
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
|