Hi,
Here's a draft patch to tackle a couple of TODOs in the RADIUS code in auth.c.
The first change is to replace select() with a standard latch loop
that responds to interrupts, postmaster death etc promptly. It's not
really too much of a big deal because the timeout was only 3 seconds
(hardcoded), but it's not good to have places that ignore ProcSignal,
and it's good to move code to our modern pattern for I/O multiplexing.
We know from experience that we have to crank timeouts up to be able
to run tests reliably on slow/valgrind/etc systems, so the second
change is to change the timeout to a GUC, as also requested by a
comment. One good side-effect is that it becomes easy and fast to
test the timed-out code path too, with a small value. While adding
the GUC I couldn't help wondering why RADIUS even needs a timeout
separate from authentication_timeout; another way to go here would be
to remove it completely, but that'd be a policy change (removing the 3
second timeout we always had). Thoughts?
The patch looks bigger than it really is because it changes the
indentation level.
But first, some basic tests to show that it works. We can test before
and after the change and have a non-zero level of confidence about
whacking the code around. Like existing similar tests, you need to
install an extra package (FreeRADIUS) and opt in with
PG_EXTRA_TESTS=radius. I figured out how to do that for our 3 CI
Unixen, so cfbot should run the tests and pass once I add this to the
March commitfest. FreeRADIUS claims to work on Windows too, but I
don't know how to set that up; maybe someday someone will fix that for
all the PG_EXTRA_TESTS tests. I've also seen this work on a Mac with
MacPorts. There's only one pathname in there that's a wild guess:
non-Debianoid Linux systems; if you know the answer there please LMK.
