[myserver-commit] [2889] Refactoring for `Cgi::send()'.

[Giuseppe Scrivano]

Revision: 2889
          http://svn.sv.gnu.org/viewvc/?view=rev&root=myserver&revision=2889
Author:   gscrivano
Date:     2008-10-18 20:38:11 +0000 (Sat, 18 Oct 2008)

Log Message:
-----------
Refactoring for `Cgi::send()'.  Added `Pipe::waitForData()', some parts of code 
was changed to use it.

Modified Paths:
--------------
    trunk/myserver/include/base/pipe/pipe.h
    trunk/myserver/include/http_handler/cgi/cgi.h
    trunk/myserver/src/base/pipe/pipe.cpp
    trunk/myserver/src/connections_scheduler/connections_scheduler.cpp
    trunk/myserver/src/http_handler/cgi/cgi.cpp
    trunk/myserver/src/server/clients_thread.cpp
    trunk/myserver/tests/test_pipe.cpp

Modified: trunk/myserver/include/base/pipe/pipe.h
===================================================================
--- trunk/myserver/include/base/pipe/pipe.h     2008-10-18 16:40:54 UTC (rev 
2888)
+++ trunk/myserver/include/base/pipe/pipe.h     2008-10-18 20:38:11 UTC (rev 
2889)
@@ -40,6 +40,7 @@
        void closeRead();
        void closeWrite();
        bool pipeTerminated(){return terminated;}
+  int waitForData (int sec, int usec);
 private:
   bool terminated;
 #ifdef NOT_WIN

Modified: trunk/myserver/include/http_handler/cgi/cgi.h
===================================================================
--- trunk/myserver/include/http_handler/cgi/cgi.h       2008-10-18 16:40:54 UTC 
(rev 2888)
+++ trunk/myserver/include/http_handler/cgi/cgi.h       2008-10-18 20:38:11 UTC 
(rev 2889)
@@ -24,15 +24,24 @@
 #include <include/protocol/http/http_headers.h>
 #include <include/protocol/http/http_data_handler.h>
 
+class Pipe;
+class FiltersChain;
+
 class Cgi : public HttpDataHandler
 {
-  static int cgiTimeout;
 public:
-  static void setTimeout(int);
-  static int getTimeout();
-       virtual int send(HttpThreadContext*, ConnectionPtr s,
+  static void setTimeout (int);
+  static int getTimeout ();
+       virtual int send (HttpThreadContext*, ConnectionPtr s,
                    const char* scriptpath, const char* exec = 0,
                    int execute = 0, int onlyHeader = 0);
+private:
+  int sendData (HttpThreadContext* td, Pipe &stdOutFile, FiltersChain& chain, 
+                Process& cgiProc, int onlyHeader, bool nph);
+  int sendHeader (HttpThreadContext* td, Pipe &stdOutFile, FiltersChain& 
chain, 
+                  Process& cgiProc, int onlyHeader, bool nph, u_long 
procStartTime, 
+                  bool keepalive, bool useChunks, int *ret);
+
+  static int cgiTimeout;
 };
 #endif
-

Modified: trunk/myserver/src/base/pipe/pipe.cpp
===================================================================
--- trunk/myserver/src/base/pipe/pipe.cpp       2008-10-18 16:40:54 UTC (rev 
2888)
+++ trunk/myserver/src/base/pipe/pipe.cpp       2008-10-18 20:38:11 UTC (rev 
2889)
@@ -98,7 +98,7 @@
 #ifdef NOT_WIN
 
 #ifdef HAVE_PIPE
-  return pipe(handles);
+  return pipe (handles);
 #else
   return 1;
 #endif
@@ -271,3 +271,45 @@
     writeHandle = 0;
 #endif
 }
+
+/*!
+ *Wait until new data is ready.  Do not wait more 
+ *than the specified timeout.
+ *\param sec Seconds part of the timeout.
+ *\param sec Micro seconds part of the timeout.
+ */
+int Pipe::waitForData (int sec, int usec)
+{
+#ifdef NOT_WIN
+
+#if HAVE_PIPE
+  struct timeval tv;
+  fd_set readfds;
+  int ret;
+  tv.tv_sec = sec;
+  tv.tv_usec = usec;
+
+  FD_ZERO(&readfds);
+
+  FD_SET(handles[0], &readfds);
+
+  ret = ::select(handles[0] + 1, &readfds, NULL, NULL, &tv);
+
+  if(ret == -1 || ret == 0)
+  {
+    return 0;
+  }
+
+  if (FD_ISSET(handles[0], &readfds))
+    return 1;
+
+  return 0;
+#else
+  return 0;
+#endif
+
+#else
+  return WaitForSingleObject (readHandle, sec * 1000 + usec / 1000) == 
WAIT_OBJECT_0;
+#endif
+
+}

Modified: trunk/myserver/src/connections_scheduler/connections_scheduler.cpp
===================================================================
--- trunk/myserver/src/connections_scheduler/connections_scheduler.cpp  
2008-10-18 16:40:54 UTC (rev 2888)
+++ trunk/myserver/src/connections_scheduler/connections_scheduler.cpp  
2008-10-18 20:38:11 UTC (rev 2889)
@@ -546,7 +546,7 @@
   dispatcherArg.terminate = true;
 
   if(server)
-    max = server->getNumThreads() * 10;
+    max = server->getNumThreads() * 2;
 
   for(u_long i = 0; i < max; i++)
   {

Modified: trunk/myserver/src/http_handler/cgi/cgi.cpp
===================================================================
--- trunk/myserver/src/http_handler/cgi/cgi.cpp 2008-10-18 16:40:54 UTC (rev 
2888)
+++ trunk/myserver/src/http_handler/cgi/cgi.cpp 2008-10-18 20:38:11 UTC (rev 
2889)
@@ -66,25 +66,16 @@
    *Use this flag to check if the CGI executable is 
    *nph (Non Parsed Header).  
    */
-  int nph = 0;
+  bool nph = false;
   ostringstream cmdLine;
-  u_long nbw = 0;
-  u_long nbw2 = 0;
 
   FiltersChain chain;
   Process cgiProc;
-  u_long procStartTime;
 
   StartProcInfo spi;
   string moreArg;
   string tmpCgiPath;
   string tmpScriptPath;
-  u_long nBytesRead;
-  u_long headerSize = 0;
-  bool useChunks = false;
-  bool keepalive = false;
-  bool headerCompleted = false;
-  u_long headerOffset = 0;
 
   /*!
    *Standard CGI uses STDOUT to output the result and the STDIN 
@@ -92,16 +83,14 @@
    */
   Pipe stdOutFile;
   File stdInFile;
-  int subString = cgipath[0] == '"';
-  int len = strlen(cgipath);
+  int len = strlen (cgipath);
   int i;
 
-  td->scriptPath.assign(scriptpath);
+  td->scriptPath.assign (scriptpath);
   
   
+  int subString = cgipath[0] == '"';
   /* Do not modify the text between " and ".  */
-
-  /* Are we in a string block?  */
   for(i = 1; i < len; i++)
   {
     if(!subString && cgipath[i] == ' ')
@@ -110,8 +99,6 @@
       subString = !subString;
   }
 
-  checkDataChunks(td, &keepalive, &useChunks);
-
   /*
    *Save the cgi path and the possible arguments.
    *the (x < len) case is when additional arguments are specified. 
@@ -151,27 +138,23 @@
       args = &td->pathInfo[1];
     
     if(cgipath && strlen(cgipath))
-      cmdLine << td->cgiRoot << "/" << td->cgiFile << " " << moreArg << " " 
+      cmdLine << tmpCgiPath << " " << moreArg << " " 
               << td->scriptFile <<  (args ? args : "" ) ;
     else
-      cmdLine << td->scriptDir << "/" << td->scriptFile << " " 
-              << moreArg << " " << (args ? args : "" );
+      cmdLine << tmpScriptPath << moreArg << " " << (args ? args : "" );
 
-    if(td->scriptFile.length() > 4 && td->scriptFile[0] == 'n'
-       && td->scriptFile[1] == 'p' && td->scriptFile[2] == 'h' 
-       && td->scriptFile[3] == '-' )
-      nph = 1; 
-    else
-      nph = 0;
+    nph = td->scriptFile.length() > 4 && td->scriptFile[0] == 'n'
+      && td->scriptFile[1] == 'p' && td->scriptFile[2] == 'h' 
+      && td->scriptFile[3] == '-' ;
 
     if(cgipath && strlen(cgipath))
     {
-      spi.cmd.assign(td->cgiRoot);
-      spi.cmd.append("/");
-      spi.cmd.append(td->cgiFile);
+      spi.cmd.assign(tmpCgiPath);
+      spi.arg.append(" ");
       spi.arg.assign(moreArg);
       spi.arg.append(" ");
       spi.arg.append(td->scriptFile);
+
       if(args)
       {
         spi.arg.append(" ");
@@ -240,9 +223,9 @@
     if(td->cgiFile.length() > 4 && td->cgiFile[0] == 'n'  
        && td->cgiFile[1] == 'p' && td->cgiFile[2] == 'h' 
        && td->cgiFile[3] == '-' )
-      nph = 1;
+      nph = true;
     else
-      nph = 0;
+      nph = false;
   }
 
   /*
@@ -267,18 +250,12 @@
   }
   
   /*
-   *Build the environment string used by the CGI started
-   *by the execHiddenProcess(...) function.
+   *Build the environment string used by the CGI process.
    *Use the td->buffer2 to build the environment string.
    */
   (td->buffer2->getBuffer())[0] = '\0';
   Env::buildEnvironmentString(td, td->buffer2->getBuffer());
   
-  /*
-   *With this code we execute the CGI process.
-   *Fill the StartProcInfo struct with the correct values and use it
-   *to run the process.
-   */
   spi.cmdLine = cmdLine.str();
   spi.cwd.assign(td->scriptDir);
 
@@ -312,52 +289,166 @@
     stdOutFile.closeWrite();  
   }
 
+  int ret = sendData (td, stdOutFile, chain, cgiProc, onlyHeader, nph);
+
+  stdOutFile.close();
+  stdInFile.close();
+  cgiProc.terminateProcess();
+  chain.clearAllFilters(); 
+
+
+  chain.clearAllFilters();   
+
+  cgiProc.terminateProcess();
+
+  /* Delete the file only if it was created by the CGI module.  */
+  if(!td->inputData.getHandle())
+    FilesUtility::deleteFile(td->inputDataPath.c_str());
+
+  return ret;
+}
+
+/*
+ *Read data from the CGI process and send it back to the client.
+ */
+int Cgi::sendData (HttpThreadContext* td, Pipe &stdOutFile, FiltersChain& 
chain, 
+                   Process &cgiProc, int onlyHeader, bool nph)
+{
+  u_long nbw = 0;
+  u_long nbw2 = 0;
+  u_long nBytesRead = 0;
+  u_long procStartTime;
+  bool useChunks = false;
+  bool keepalive = false;
+  int ret = 0;
+
   /* Reset the buffer2 length counter. */
   td->buffer2->setLength(0);
 
-  /* Read the CGI output.  */
-  nBytesRead = 0;
-
   procStartTime = getTicks();
 
-  headerSize = 0;
+  checkDataChunks(td, &keepalive, &useChunks);
 
+  if (sendHeader (td, stdOutFile, chain, cgiProc, onlyHeader, nph, 
procStartTime, keepalive, useChunks, &ret))
+  {
+    return ret;
+  }
+
+  if(!nph && onlyHeader)
+  {
+    return 1;
+  }
+
+
+  /* Create the output filters chain.  */
+  if(td->mime && Server::getInstance()->getFiltersFactory()->chain(&chain,
+                                                                   
td->mime->filters, 
+                                                                   
td->connection->socket, 
+                                                                   &nbw, 
+                                                                   1))
+  {
+    td->connection->host->warningsLogWrite("Cgi: Error loading filters");
+    return td->http->raiseHTTPError(500);
+  }
+
+  if (td->response.getStatusType () == HttpResponseHeader::SUCCESSFUL)
+  {
+    /* Send the rest of the data until we can read from the pipe.  */
+    for (;;)
+    {
+      nBytesRead = 0;
+      int aliveProcess = 0;
+      u_long ticks = getTicks() - procStartTime;
+
+      if (ticks >= cgiTimeout)
+        break;
+
+      if (stdOutFile.waitForData ((cgiTimeout - ticks) / 1000, (cgiTimeout - 
ticks) % 1000) == 0)
+        break;
+
+      aliveProcess = !(stdOutFile.pipeTerminated ());
+
+      /* Read data from the process standard output file.  */
+      if (stdOutFile.read(td->buffer2->getBuffer (), 
+                          td->buffer2->getRealLength (), 
+                          &nBytesRead))
+      {
+        return 0;      
+      }
+      
+      if (!aliveProcess && !nBytesRead)
+        break;
+      
+      if (nBytesRead && 
+          HttpDataHandler::appendDataToHTTPChannel (td, 
+                                                    td->buffer2->getBuffer(),
+                                                    nBytesRead,
+                                                    &(td->outputData),
+                                                    &chain,
+                                                    td->appendOutputs, 
+                                                    useChunks))
+      {
+        return 0;       
+      }
+      
+      nbw += nBytesRead;
+    }
+  
+  
+    /* Send the last null chunk if needed.  */
+    if(useChunks && chain.write("0\r\n\r\n", 5, &nbw2))
+    {
+      return 0;       
+    }
+  }
+
+  /* Update the Content-Length field for logging activity.  */
+  td->sentData += nbw;
+  
+  return 1;  
+}
+
+/*!
+ *Send the HTTP header.
+ *\return nonzero if the reply is already complete.
+ */
+int Cgi::sendHeader (HttpThreadContext* td, Pipe &stdOutFile, FiltersChain& 
chain, 
+                     Process& cgiProc, int onlyHeader, bool nph, u_long 
procStartTime, 
+                     bool keepalive, bool useChunks, int *ret)
+{
+  u_long headerSize = 0;
+  bool headerCompleted = false;
+  u_long headerOffset = 0;
+  u_long nbw = 0;
+  u_long nBytesRead;
   /* Parse initial chunks of data looking for the HTTP header.  */
   while(!headerCompleted && !nph)
   {
+    u_long ticks = getTicks() - procStartTime;
     bool term;
+    nBytesRead = 0;
     /* Do not try to read using a small buffer as this has some
        bad influence on the performances.  */
     if(td->buffer2->getRealLength() - headerOffset - 1 < 512)
       break;
     
-    nBytesRead = 0;
-    
     term = stdOutFile.pipeTerminated();
-      
-    if(stdOutFile.read(td->buffer2->getBuffer() + headerOffset, 
-                       td->buffer2->getRealLength() - headerOffset - 1, 
-                       &nBytesRead))
+
+    if (stdOutFile.waitForData ((cgiTimeout - ticks) / 1000, (cgiTimeout - 
ticks) % 1000) == 0)
+      break;
+
+    if (stdOutFile.read (td->buffer2->getBuffer() + headerOffset, 
+                         td->buffer2->getRealLength() - headerOffset - 1, 
+                         &nBytesRead))
     {
-      stdInFile.close();
-      stdOutFile.close();
-      td->connection->host->warningsLogWrite
-        ("Cgi: Error reading from CGI std out file");
-      chain.clearAllFilters();
-      return td->http->raiseHTTPError(500);
+      *ret = td->http->raiseHTTPError(500);
+      return 1;
     }
       
-    if(nBytesRead == 0)
+    if (nBytesRead == 0 && term)
     {
-      if((int)(getTicks() - procStartTime) > cgiTimeout)
-         break;
-      else
-      {
-        if(term)
-          break;
-        
-        continue;
-      }
+      *ret = td->http->raiseHTTPError(500);
+      return 1;
     }
 
     headerOffset += nBytesRead;
@@ -367,17 +458,10 @@
     
     if(headerOffset == 0)
     {
-      td->connection->host->warningsLogWrite("Cgi: Error CGI zero bytes read");
-      td->http->raiseHTTPError(500);
-      stdOutFile.close();
-      stdInFile.close();
-      chain.clearAllFilters(); 
-      cgiProc.terminateProcess();
-      return 0;
+      *ret = td->http->raiseHTTPError(500);
+      return 1;
     }
 
-    /* Standard CGI can include an extra HTTP header.  */
-    nbw = 0;
     for(u_long i = std::max(0, (int)headerOffset - (int)nBytesRead - 10); 
         i < headerOffset; i++)
     {
@@ -431,11 +515,7 @@
        */
       if(location && location->length())
       {
-        td->http->sendHTTPRedirect(location->c_str());
-        stdOutFile.close();
-        stdInFile.close();
-        chain.clearAllFilters(); 
-        cgiProc.terminateProcess();
+        *ret = td->http->sendHTTPRedirect(location->c_str());
         return 1;
       }
 
@@ -444,44 +524,15 @@
 
       td->buffer->setLength((u_int)strlen(td->buffer->getBuffer()));
 
-      if(chain.write(td->buffer->getBuffer(),
-                     static_cast<int>(td->buffer->getLength()), &nbw2))
+      if (chain.write(td->buffer->getBuffer(),
+                      static_cast<int>(td->buffer->getLength()), &nbw))
       {
-        stdInFile.close();
-        stdOutFile.close();
-        chain.clearAllFilters(); 
-        /* Remove the connection on sockets error.  */
-        cgiProc.terminateProcess();
-        return 0;
+        *ret = 0;
+        return 1;
       }
     }
   }
 
-  if(!nph && onlyHeader)
-  {
-    stdOutFile.close();
-    stdInFile.close();
-    chain.clearAllFilters(); 
-    cgiProc.terminateProcess();
-    return 1;
-  }
-
-
-  /* Create the output filters chain.  */
-  if(td->mime && Server::getInstance()->getFiltersFactory()->chain(&chain,
-                                                                   
td->mime->filters, 
-                                                                   
td->connection->socket, 
-                                                                   &nbw, 
-                                                                   1))
-  {
-    td->connection->host->warningsLogWrite("Cgi: Error loading filters");
-    stdOutFile.close();
-    stdInFile.close();
-    cgiProc.terminateProcess();
-    chain.clearAllFilters(); 
-    return td->http->raiseHTTPError(500);
-  }
-
   if(headerOffset - headerSize)
   {
     /* Flush the buffer.  Data from the header parsing can be present.  */
@@ -493,103 +544,14 @@
                                                 td->appendOutputs, 
                                                 useChunks))
     {
-      stdOutFile.close();
-      stdInFile.close();
-      chain.clearAllFilters(); 
-      /* Remove the connection on sockets error.  */
-      cgiProc.terminateProcess();
       return 0;    
     }
 
-    nbw += headerOffset - headerSize;
+    td->sentData += headerOffset - headerSize;
   }
 
-  if (td->response.getStatusType () == HttpResponseHeader::SUCCESSFUL)
-  {
-    /* Send the rest of the data until we can read from the pipe.  */
-    for(;;)
-    {
-      int aliveProcess = 0;
 
-      /* Process timeout.  */
-      if((int)(getTicks() - procStartTime) > cgiTimeout)
-      {
-        stdOutFile.close();
-        stdInFile.close();
-        chain.clearAllFilters(); 
-        /* Remove the connection on sockets error.  */
-        cgiProc.terminateProcess();
-        return 0;       
-      }
-    
-      aliveProcess = cgiProc.isProcessAlive();
-
-      /* Read data from the process standard output file.  */
-      if(stdOutFile.read(td->buffer2->getBuffer(), 
-                         td->buffer2->getRealLength(), 
-                         &nBytesRead))
-      {
-        stdOutFile.close();
-        stdInFile.close();
-        chain.clearAllFilters(); 
-        /* Remove the connection on sockets error.  */
-        cgiProc.terminateProcess();
-        return 0;      
-      }
-      
-      if(!aliveProcess && !nBytesRead)
-        break;
-      
-      if(nBytesRead && 
-         HttpDataHandler::appendDataToHTTPChannel(td, 
-                                                  td->buffer2->getBuffer(),
-                                                  nBytesRead,
-                                                  &(td->outputData),
-                                                  &chain,
-                                                  td->appendOutputs, 
-                                                  useChunks))
-      {
-        stdOutFile.close();
-        stdInFile.close();
-        chain.clearAllFilters(); 
-        /* Remove the connection on sockets error.  */
-        cgiProc.terminateProcess();
-        return 0;       
-      }
-      
-      nbw += nBytesRead;
-    }
-  
-  
-    /* Send the last null chunk if needed.  */
-    if(useChunks && chain.write("0\r\n\r\n", 5, &nbw2))
-    {
-      stdOutFile.close();
-      stdInFile.close();
-      chain.clearAllFilters(); 
-  
-      /* Remove the connection on sockets error.  */
-      cgiProc.terminateProcess();
-      return 0;       
-    }
-  }
-
-  /* Update the Content-Length field for logging activity.  */
-  td->sentData += nbw;
-
-  chain.clearAllFilters();   
-
-  cgiProc.terminateProcess();
-
-  /* Close the stdin and stdout files used by the CGI.  */
-  stdOutFile.close();
-  stdInFile.close();
-
-  /* Delete the file only if it was created by the CGI module.  */
-  if(!td->inputData.getHandle())
-    FilesUtility::deleteFile(td->inputDataPath.c_str());
-  
-  return 1;  
+  return 0;
 }
 
 /*!

Modified: trunk/myserver/src/server/clients_thread.cpp
===================================================================
--- trunk/myserver/src/server/clients_thread.cpp        2008-10-18 16:40:54 UTC 
(rev 2888)
+++ trunk/myserver/src/server/clients_thread.cpp        2008-10-18 20:38:11 UTC 
(rev 2889)
@@ -187,7 +187,7 @@
        */
       if((!ct->isStatic()) && ct->isToDestroy())
       {
-        Thread::wait(1);
+        Thread::wait(1000);
         continue;
       }
 

Modified: trunk/myserver/tests/test_pipe.cpp
===================================================================
--- trunk/myserver/tests/test_pipe.cpp  2008-10-18 16:40:54 UTC (rev 2888)
+++ trunk/myserver/tests/test_pipe.cpp  2008-10-18 20:38:11 UTC (rev 2889)
@@ -30,6 +30,7 @@
   CPPUNIT_TEST( testCreateClose ); 
   CPPUNIT_TEST( testWriteRead );
   CPPUNIT_TEST( testInverted );
+  CPPUNIT_TEST( testWaitForData );
   CPPUNIT_TEST_SUITE_END();
 
   Pipe *pipe;
@@ -87,7 +88,27 @@
     pipe->close();
   }
 
+  void testWaitForData()
+  {
+    char outBuff[256];
+    char inBuff[256];
+    u_long nbw;
+    u_long nbr;
 
+    strcpy(outBuff, "MyServer is a powerful and easy to configure web server");
+
+    int ret = pipe->create();
+
+    CPPUNIT_ASSERT_EQUAL(pipe->waitForData(0, 100), 0);
+
+    ret = pipe->write(outBuff, 256, &nbw);
+
+    CPPUNIT_ASSERT_EQUAL(pipe->waitForData(0, 100), 1);
+
+    pipe->close();
+  }
+
+
   void testInverted()
   {
     Pipe pipeInv;