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 2002/08/26 16:31:38 UTC

DO NOT REPLY [Bug 12041] New: - CGIServlet can block on input

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=12041>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=12041

CGIServlet can block on input

           Summary: CGIServlet can block on input
           Product: Tomcat 4
           Version: 4.1.9
          Platform: Sun
        OS/Version: Solaris
            Status: NEW
          Severity: Major
          Priority: Other
         Component: Servlets:CGI
        AssignedTo: tomcat-dev@jakarta.apache.org
        ReportedBy: dglo@ssec.wisc.edu


I've got some legacy Perl CGI scripts which I wanted to run under Tomcat 4.1.x. 
After some debugging I discovered that the CGIServlet code would first try to
read all the stderr input before trying to read stdout.

This doesn't work on at least Solaris 7 under JDK 1.3, because the stderr handle
doesn't seem to get closed, so the code hangs trying to read stderr.

However, this approach seems like it would have 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.

The patch below seems to get around 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.

Here's the patch:

Index: CGIServlet.java
===================================================================
RCS file: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catal
ina/servlets/CGIServlet.java,v
retrieving revision 1.7
diff -u -3 -p -r1.7 CGIServlet.java
--- CGIServlet.java     20 Sep 2001 23:48:13 -0000      1.7
+++ CGIServlet.java     26 Aug 2002 14:27:27 -0000
@@ -1663,7 +1663,6 @@ public class CGIServlet extends HttpServ
              *                               bugParade/bugs/4223650.html
              */
 
-            boolean isRunning = true;
             commandsStdOut = new BufferedReader
                 (new InputStreamReader(proc.getInputStream()));
             commandsStdErr = new BufferedReader
@@ -1680,64 +1679,94 @@ public class CGIServlet extends HttpServ
                 //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 + "\")");
+                } 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 {
+                            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 + "\"
");
+                            }
                         }
-                        if (line.startsWith("HTTP")) {
-                            //TODO: should set status codes (NPH support)
-                            /*
-                             * response.setStatus(getStatusCode(line));
-                             */
+                    } else {
+                        //write output
+                        bufRead = commandsStdOut.read(cBuf);
+                        if (bufRead == -1) {
+                            commandsStdOut.close();
+                            commandsStdOut = null;
+                            isRunning = (commandsStdErr != null);
                         } else {
-                            response.addHeader
-                                (line.substring(0, line.indexOf(":")).trim(),
-                                 line.substring(line.indexOf(":") + 1).trim());
+                            if (servletContainerStdout != null) {
+                                if (debug >= 4) {
+                                    log("runCGI: write(\"" + new String(cBuf, 0
, bufRead) + "\")");
+                                }
+                                servletContainerStdout.write(cBuf, 0, bufRead);
+                            }
                         }
                     }
-
-                    //write output
-                    cBuf = new char[1024];
-                    while ((bufRead = commandsStdOut.read(cBuf)) != -1) {
-                        if (servletContainerStdout != null) {
-                            if (debug >= 4) {
-                                log("runCGI: write(\"" + cBuf + "\")");
+                } else {
+                    try {
+                        int exitVal = proc.exitValue();
+                        pauseCount++;
+                        if (pauseCount > 2) {
+                            isRunning = false;
+                        } else {
+                            // pause for half a second
+                            try {
+                                Thread.sleep(500);
+                            } catch (InterruptedException ie) {
                             }
-                            servletContainerStdout.write(cBuf, 0, bufRead);
                         }
+                    } catch (IllegalThreadStateException ex) {
                     }
+                }
 
-                    if (servletContainerStdout != null) {
-                        servletContainerStdout.flush();
 
-                    if (servletContainerStdout != null) {
-                        servletContainerStdout.flush();
-                    }
-
-                    proc.exitValue(); // Throws exception if alive
-
-                    isRunning = false;
-
-                } catch (IllegalThreadStateException e) {
-                    try {
-                        Thread.currentThread().sleep(500);
-                    } catch (InterruptedException ignored) {
-                    }
+                if (servletContainerStdout != null) {
+                    servletContainerStdout.flush();
                 }
             } //replacement for Process.waitFor()
-
-
         }
 
 
And since web forms tend to hose patches, here's a gzip'd uuencoded version of
the same patch:

begin 644 CGIServlet.patch.gz
M'XL(",DZ:CT  V-S+G!A=&-H ,U846_;-A!^KG_%U0^%%-N2[+A)ZR!MUZQ;
M @Q-D 3;P[8'6J(M=;2DD92=H,U_WU&274D6)05)@5V".!;)X_&^CY^.? _S
M)&">)?S>>W!%^L>*B735]R 4DC"6-49\V;L(/7HW@[-?+VXH7S,JK2]D37JG
M3[?>]=D-+ )&9V#[T8K:[EK$R9P%KOV%_$.X)",9K5PB1U/+L?&3L" DMN"N
M+7S"J8WAV20FKD\+K5F0PJX$/%SW.)4\H.L@7 +'#Q%$(8RMXYX7+!8P2F!T
M"*,81AR?[2UW-!I5G[V8.'!#8Y@XSA@FA[/IF]D8/3AH+Y3;P6"P/^0(?DJ6
M:L@$QM/9Y!A_LR&]#Q]@-#XZ.AP>PR#]/ )\E.4#7$:$*'@#>B=IZ DXES)6
MSWI0L@-HMGFRO"*<>-3&_X0]G4P.CUX[EB]7K.K)[D%O5!H;18R2$ )QG82A
MRN8I2)[0D_)(-UJM"(9X([W+1&*?D&[@8[)84$Z]:XIS\\I4:(;J=!'&B;R1
MG))5UL^(>>1:2RH++89IFOH9/W%>/V.6Y3?.\&BJTGS\=OAV^H0\H]GVY\O+
MJQF$$42)Q/A@$S &<PH;'D@<71[R@.D<U*8S/$]#W&6SU"L()<0D$?0L2D*5
M30<[:!PUX[+Q<<>!L>MEPM<JPLHDO\>&O<?9BA$ #X3$:#EN8"YD?4<7DY_#
MX.*&_7/L3*9_G]3WS<,RYLE"087#RG!::DY#>31->'F***K !U4WP0*,"@^P
M<Y@@(J]>U;F\1R+5.<H6"F2.H(*,(%LRJI3T,6DG^%W0'!)TBYC4NVC!;&L=
M5GU2/U(M>#=:FY8=(F7O+HL$-72N]P= ELJ&_D7R&14)R('03?< E&$RO^YO
ML>):<WT_BT))@I!R]!T5?.LXN[7ZX9;:JC3-\Q"<X1:.IL1LX]ENV\:T_Y#9
M=_EJG99%2Z//DQ!U;0:W/H\V"B"R(?>0X_I7OP\::E9-[6:48/2P'S$,H(^N
M6B/70ZQI>NB-=%JDMJ&?0B#JNV3! E8'=,??1@DR\JYE^J9;\3=L,3(!2NG6
M2#84G)=&OV_1?Q/"1.K5-!L9J@CE47PEP[M3F+22N80K\;R,B48*9K;>% ^S
M%I"</A7!+.S3BF!N4_#_$,S.6Z],$&VW,DFTR#<I'T:4#<_3]^T;5,%OVZR%
M&F"!HZI%0-&Z[?Y]0CU.+!Y+JE*$[:$I=Q:>-[@4?P32-_KGM[=7_?8\*;/M
MV\N?+V<@_"AAR#1$&3W)1"!Z'A5@?+XZ!Y'$<<2EV<'=00?Y.U LCJ-08-04
M"U$UG;'<_G>&\^9 MZ0F<V9WTO==G@)U$+M<&/U9WU1P.IW2M MX!V4WF<^Q
M2>8BTWI4^;TH3 O;L!(?=G,)%9?[JQK >.OS![W]YM$2&9+)0<;F*K&?]O9J
ME/8&NC?J_..HWNSJH+G],11O<:2A=SMPMIT61/EQ2M^OMFC>ZG5CT:SL<86S
MLLHL[<7S_J#V CH-35-$%TXSU;-OT789;@2H1AB: 87'BD*KNV<5A&Z'@U:U
M*+TQIYV&I"LIBDQ6SV?"TEXNM[])E;6\394][^GB.6OTTG[^L5<%Y?U?N"IH
M5.8GGBKW2=/*?2UATDRTU^T:7F;7-GK1"]655B!_)PS3EEZKY5^39BG[7I8/
M!BVB6JC@WW6H.8MB]SQ%+YY"LF/&(N+@$[8 @GO#C4*O>5Q+[K:&!VCDER48
MI;'QVG':*Q57W:V#<1%*RGD22^I]NG-I+-4%=$#-IBN/U,'S7FDTS*-Y86_C
M9XPN"<O6K^H!^GT9]$Z[C!JW#W4WCLJ>N!<UJ5BP1/B&KF+1ZM;>]E#,2J]/
M!*YW!R#2BP5KJO-2P^^:GMV2K%U\PVVMLIRR;L(Y#67VS3!+%*Y/33-UEV'$
MJ:<-J@;X)[RDV["MF;ZR%G5O'3/BTA4F(16'*T28"F%M2"!_B;AA*FAZ10_X
*\Q__WM3<+QL  +"M
 
end

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>