Hi, On 2025-02-20 09:28:36 -0800, Jacob Champion wrote: > On Thu, Feb 20, 2025 at 8:30 AM Daniel Gustafsson <dan...@yesql.se> wrote: > > I spent a few more hours staring at this, and ran it through a number of CI > > and > > local builds, without anything showing up. Pushed to master with the first > > set > > of buildfarm animals showing green builds. > > Thank you!! And _huge thanks_ to everyone who's reviewed and provided > feedback. I'm going to start working with Andrew on getting the new > tests going in the buildfarm.
+1 One question about interruptability. The docs say: + <varlistentry> + <term>Interruptibility</term> + <listitem> + <para> + Modules must remain interruptible by signals so that the server can + correctly handle authentication timeouts and shutdown signals from + <application>pg_ctl</application>. For example, a module receiving + <symbol>EINTR</symbol>/<symbol>EAGAIN</symbol> from a blocking call + should call <function>CHECK_FOR_INTERRUPTS()</function> before retrying. + The same should be done during any long-running loops. Failure to follow + this guidance may result in unresponsive backend sessions. + </para> + </listitem> + </varlistentry> Is EINTR checking really sufficient? I don't think we can generally rely on all blocking system calls to be interruptible by signals on all platforms? And, probably worse, isn't relying on getting EINTR rather racy, due to the chance of the signal arriving between CHECK_FOR_INTERRUPTS() and the blocking system call? Afaict the only real way to do safely across platforms is to never call blocking functions, e.g. by using non-blocking sockets and waiting for readiness using latches. And a second one about supporting !CURL_VERSION_ASYNCHDNS: Is it a good idea to support that? We e.g. rely on libpq connections made by the backend to be interruptible. Admittedly that's, I think, already not bulletproof, due to libpq's DNS lookups going through libc if connection string contains a name that needs to be looked up, but this seems to make that a bit worse? With the connection string the DNS lookups can at least be avoided, not that I think we properly document that... Greetings, Andres
