# HG changeset patch
# User Olaf Wintermann <olaf.wintermann@gmail.com>
# Date 1716130417 -7200
# Node ID be62c9604377171ee03c5483977b8992052aa1f4
# Parent  ec22d4ccd081e6cca2f351d9ff69454a9670d412
improve cgi io event handling

diff -r ec22d4ccd081 -r be62c9604377 src/server/safs/cgi.c
--- a/src/server/safs/cgi.c	Sun May 12 11:26:59 2024 +0200
+++ b/src/server/safs/cgi.c	Sun May 19 16:53:37 2024 +0200
@@ -57,7 +57,7 @@
     char *ctlen = pblock_findkeyval(pb_key_content_length, rq->headers);
     int64_t content_length = 0;
     
-    log_ereport(LOG_DEBUG, "cgi-send: path: %s content-length: %s", path, ctlen);
+    log_ereport(LOG_DEBUG, "cgi-send: path: %s req: %p content-length: %s", path, rq, ctlen);
     
     if(ctlen) {
         if(!util_strtoint(ctlen, &content_length)) {
@@ -189,22 +189,26 @@
     
     net_setnonblock(sn->csd, 1);
     
-    // add poll events for cgi stdout/stderr
+    // add poll events for cgi stdout/stderr and netout
     int error = 0;
     if(ev_pollin(sn->ev, handler->process.err[0], stderr_readev)) {
         log_ereport(LOG_FAILURE, "send-cgi: stderr ev_pollin failed");
         error = 1;
+    } else {
+        handler->wait_read = TRUE;
+        handler->events++;
     }
-    if(ev_pollin(sn->ev, handler->process.out[0], readev)) {
+    if(!error && ev_pollin(sn->ev, handler->process.out[0], readev)) {
         log_ereport(LOG_FAILURE, "send-cgi: stdout ev_pollin failed");
         error = 1;
+    } else {
+        handler->events++;
     }
     
-    handler->wait_read = TRUE;
-    handler->events = 2; // 2 events (stdout, stderr)
+    // don't poll sn->csd yet, we wait until the first net_write fails
     
     if(error) {
-        log_ereport(LOG_FAILURE, "cgi-send: kill script: %s", path);
+        log_ereport(LOG_FAILURE, "cgi-send: initialization error: kill script: %s", path);
         kill(handler->process.pid, SIGKILL);
         cgi_parser_free(handler->parser);
         cgi_close(&handler->process);
@@ -214,6 +218,14 @@
     return REQ_PROCESSING;
 }
 
+/*
+ * Try to flush the CGIHandler write buffer
+ * 
+ * When successful, cgi_try_write_flush() returns 0. If an error occurs,
+ * 1 is returned.
+ * 
+ * If the error is not EWOULDBLOCK, handler->result is set to REQ_ABORTED.
+ */
 static int cgi_try_write_flush(CGIHandler *handler, Session *sn) {
     ssize_t wr = 0;
     while(
@@ -230,6 +242,12 @@
     if(handler->writebuf_size - handler->writebuf_pos > 0) {
         if(net_errno(sn->csd) != EWOULDBLOCK) {
             handler->result = REQ_ABORTED;
+            log_ereport(
+                    LOG_FAILURE,
+                    "cgi pid %d %s: network error: %s",
+                    (int)handler->process.pid,
+                    handler->path,
+                    strerror(net_errno(sn->csd)));
         }
         
         return 1;
@@ -237,6 +255,16 @@
     return 0;
 }
 
+/*
+ * Try to write the buffer to sn->csd
+ * In case the socket is non-blocking and not all bytes could be written,
+ * the remaining bytes are copied to the CGIHandler write buffer.
+ * 
+ * If an error occurs that is not EWOULDBLOCK, handler->result is set to
+ * REQ_ABORTED.
+ * 
+ * Returns 0 if all bytes are successfully written, otherwise 1
+ */
 static int cgi_try_write(CGIHandler *handler, EventHandler *ev, Session *sn, char *buf, size_t size) {
 
     size_t pos = 0;
@@ -262,21 +290,14 @@
             memcpy(handler->writebuf, buf+pos, remaining);
             handler->writebuf_size = remaining;
             handler->writebuf_pos = 0;
-            
-            /*
-            // initialize poll, if it isn't already active
-            if(!handler->poll_out) {
-                if(event_pollout(ev, sn->csd, handler->writeev)) {
-                    handler->result = REQ_ABORTED;
-                    return 0;
-                }
-                handler->events++;
-                handler->poll_out = TRUE;
-            }
-            */
         } else {
             handler->result = REQ_ABORTED;
-            log_ereport(LOG_FAILURE, "cgi_try_write: %s", strerror(net_errno(sn->csd)));
+            log_ereport(
+                    LOG_FAILURE,
+                    "cgi pid %d %s: network error: %s",
+                    (int)handler->process.pid,
+                    handler->path,
+                    strerror(net_errno(sn->csd)));
         }
         return 1;
     }
@@ -287,6 +308,8 @@
 int cgi_stdout_readevent(EventHandler *ev, Event *event) {
     CGIHandler *handler = event->cookie;
     
+    event->finish = cgi_event_finish;
+    handler->writeev->finish = NULL;
     CgiIOResult ret = cgi_read_output(handler, ev);
     switch(ret) {
         case CGI_IO_COMPLETE: {
@@ -296,12 +319,14 @@
             return 1;
         }
         case CGI_IO_NEED_WRITE: {
-            if(event_pollout(ev, handler->parser->sn->csd, handler->readev)) {
+            // writeev is only enabled, if needed
+            if(event_pollout(ev, handler->parser->sn->csd, handler->writeev)) {
                 handler->result = REQ_ABORTED;
             } else {
                 handler->poll_out = TRUE;
+                log_ereport(LOG_DEBUG, "cgi-send: req: %p enable poll out", handler->parser->rq);
+                return 1; // keep readevent active
             }
-            break;
         }
         case CGI_IO_ERROR: {
             break;
@@ -315,18 +340,15 @@
 int cgi_writeevent(EventHandler *ev, Event *event) {
     CGIHandler *handler = event->cookie;
     
-    // cgi_read_output will try to flush the buffer
+    event->finish = cgi_event_finish;
+    handler->readev->finish = NULL;
     CgiIOResult ret = cgi_read_output(handler, ev);
     switch(ret) {
         case CGI_IO_COMPLETE: {
             break;
         }
         case CGI_IO_NEED_READ: {
-            if(ev_pollin(ev, handler->process.out[0], event)) {
-                handler->result = REQ_ABORTED;
-            } else {
-                handler->wait_read = TRUE;
-            }
+            return 1;
         }
         case CGI_IO_NEED_WRITE: {
             return 1;
@@ -347,11 +369,14 @@
     Session *sn = parser->sn;
     Request *rq = parser->rq;
     
+    if(handler->result == REQ_ABORTED) {
+        return CGI_IO_ERROR;
+    }
+    
     // try to flush handler->writebuf
     // if writebuf is empty, this does nothing and returns 0
     if(cgi_try_write_flush(handler, sn)) {
         if(handler->result == REQ_ABORTED) {
-            log_ereport(LOG_DEBUG, "cgi-send: req: %p write failed: %s: abort", handler->parser->rq, strerror(net_errno(sn->csd)), rq);
             return CGI_IO_ERROR;
         } else {
             return CGI_IO_NEED_WRITE;
@@ -534,6 +559,11 @@
     log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: %d pollout: %d cgi_eof: %d fn: %s", rq, handler->events, handler->poll_out, handler->cgi_eof, event_fn);
     
     if(--handler->events > 0) {
+        return 0;
+    }
+    
+    /*
+    if(--handler->events > 0) {
         if(handler->events == 1) {
             if(handler->poll_out) {
                 // write event registered, however it will not be activated anymore
@@ -554,6 +584,15 @@
             return 0;
         }
     }
+    */
+    if(handler->poll_out) {
+        // write event registered, however it will not be activated anymore
+        // we can safely remove the event
+        log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: remove-poll write", rq);
+        if(event_removepoll(ev, sn->csd)) {
+            log_ereport(LOG_FAILURE, "cgi_event_finish: event_removepoll: %s", strerror(errno));
+        }
+    }
     
     if(handler->result == REQ_ABORTED && handler->process.pid != 0) {
         log_ereport(LOG_FAILURE, "cgi-send: kill script: %s", handler->path);
diff -r ec22d4ccd081 -r be62c9604377 src/server/safs/cgi.h
--- a/src/server/safs/cgi.h	Sun May 12 11:26:59 2024 +0200
+++ b/src/server/safs/cgi.h	Sun May 19 16:53:37 2024 +0200
@@ -136,12 +136,6 @@
     WSBool wait_read;
     
     /*
-     * last write returned EWOULDBLOCK
-     * waiting for the next write event
-     */
-    //WSBool wait_write;
-    
-    /*
      * cgi pipe (stdout) EOF
      */
     WSBool cgi_eof;