On 2022-06-10 04:27 -07, Alfred Morgan <[email protected]> wrote:
>> The connection to upstream (e.g. httpd) is closed so the client gets a 500
>> error.
>
> Hmm, this isn't my experience. Possibly a slowcgi bug? My clients were
> getting no response, e.g.:
> curl: (52) Empty reply from server
>
>> But the script keeps running.
>
> Ah, I see now that the process is left to run now. My CGI read/writes
> several MB/s lasting longer than the hard coded 120 sec timeout which
> means my process would immediately get a broken pipe which led me to
> erroneously believe my process was being terminated by slowcgi.
>
>> We were worried that scripts are not prepared when they suddenly get
>> terminated.
>
> To put worry at rest, in the rfc 3875 CGI Version 1.1 section 3.4 Execution:
> ...
> "In the event of an error condition, the server can interrupt or
> terminate script execution at any time and without warning. That
> could occur, for example, in the event of a transport failure between
> the server and the client; so the script SHOULD be prepared to handle
> abnormal termination."
>
>> Also as sthen pointed out your MUA mangled the diff.
>
> I believe I mangled the diff by copying from my terminal window.
>
>> the man page needs better wording.
>> please use strtonum(3) and use the idiom from the man page example.
>
> I will attach the patch.
>
> -alfred
>
> 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.
> .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
> +.Ar timeout
> +seconds instead of the default 120 seconds. The CGI is left to run.
> .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",
> __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.
> /*
> * 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);
> + 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);
> }
>
> void
>
--
I'm not entirely sure you are real.
