Sun, 19 May 2024 16:53:37 +0200
improve cgi io event handling
src/server/safs/cgi.c | file | annotate | diff | comparison | revisions | |
src/server/safs/cgi.h | file | annotate | diff | comparison | revisions |
--- 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);
--- 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;