after the other thread on tech@ reminded me this is still pending, i
took a look, tested and committed it with some minor tweaks.

Thanks!

Florian Obser <[email protected]> wrote:
> On 2022-06-10 04:27 -07, Alfred Morgan <[email protected]> wrote:
> > Index: slowcgi.8
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.8,v
> > retrieving revision 1.16
> > diff -u -p -r1.16 slowcgi.8
> > --- slowcgi.8       2 Sep 2021 14:14:44 -0000       1.16
> > +++ slowcgi.8       10 Jun 2022 11:20:04 -0000
> > @@ -76,6 +76,10 @@ effectively disables the chroot.

i've mentioned the -t flag in the SYNOPSIS.

> >  .It Fl s Ar socket
> >  Create and bind to alternative local socket at
> >  .Ar socket .
> > +.It Fl t Ar timeout
> > +Closes the file descriptors after

All other descriptions uses the imperative, also

> > +.Ar timeout
> > +seconds instead of the default 120 seconds. The CGI is left to run.

new sentence, new line.

I changed the description also to be (hopefully) clearer: what "file
descriptors" will be closed?

I went with

     -t timeout
             Terminate the request after timeout seconds instead of the
             default 120 seconds.  The CGI script is left to run but its
             standard input, output and error will be closed.

> >  .It Fl U Ar user
> >  Change the owner of
> >  .Pa /var/www/run/slowcgi.sock
> > Index: slowcgi.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.c,v
> > retrieving revision 1.62
> > diff -u -p -r1.62 slowcgi.c
> > --- slowcgi.c       2 Sep 2021 14:14:44 -0000       1.62
> > +++ slowcgi.c       10 Jun 2022 11:20:04 -0000
> > @@ -40,6 +40,7 @@
> >  #include <unistd.h>
> >  
> >  #define TIMEOUT_DEFAULT             120
> > +#define TIMEOUT_MAX                 86400 * 365
> >  #define SLOWCGI_USER                "www"
> >  
> >  #define FCGI_CONTENT_SIZE   65535
> > @@ -252,7 +253,7 @@ usage(void)
> >  {
> >     extern char *__progname;
> >     fprintf(stderr,
> > -       "usage: %s [-dv] [-p path] [-s socket] [-U user] [-u user]\n",
> > +       "usage: %s [-dv] [-p path] [-s socket] [-t timeout] [-U user] [-u 
> > user]\n",

line >80 chars, folded

> >         __progname);
> >     exit(1);
> >  }
> > @@ -275,6 +276,7 @@ main(int argc, char *argv[])
> >     const char      *chrootpath = NULL;
> >     const char      *sock_user = SLOWCGI_USER;
> >     const char      *slowcgi_user = SLOWCGI_USER;
> > +   const char *errstr;
> >
>                   ^ space vs. tab
> 
> With that fixed this is OK florian
> 
> No need to resend the diff, I trust whoever ends up commiting this can
> fix that up before commit.

done :)

> >     /*
> >      * Ensure we have fds 0-2 open so that we have no fd overlaps
> > @@ -293,7 +295,7 @@ main(int argc, char *argv[])
> >             }
> >     }
> >  
> > -   while ((c = getopt(argc, argv, "dp:s:U:u:v")) != -1) {
> > +   while ((c = getopt(argc, argv, "dp:s:t:U:u:v")) != -1) {
> >             switch (c) {
> >             case 'd':
> >                     debug++;
> > @@ -304,6 +306,11 @@ main(int argc, char *argv[])
> >             case 's':
> >                     fcgi_socket = optarg;
> >                     break;
> > +           case 't':
> > +                   timeout.tv_sec = strtonum(optarg, 1, TIMEOUT_MAX, 
> > &errstr);

folded too

> > +                   if (errstr != NULL)
> > +                           errx(1, "timeout is %s: %s", errstr, optarg);
> > +                   break;
> >             case 'U':
> >                     sock_user = optarg;
> >                     break;
> > @@ -507,7 +514,12 @@ slowcgi_accept(int fd, short events, voi
> >  void
> >  slowcgi_timeout(int fd, short events, void *arg)
> >  {
> > -   cleanup_request((struct request*) arg);
> > +   struct request          *req;
> > +
> > +   req = arg;
> > +
> > +   lwarnx("timeout child %i", req->script_pid);
> > +   cleanup_request(req);

I skipped this hunk.

(fwiw, I have some doubts on how slowcgi_timeout works, as it doesn't
correctly shut down the requests from the FastGCI pov -- httpd still
hangs a bit on slowcgi timeouts -- but this diff doesn't make it worse
anyway so i went ahead and committed it.)

Reply via email to