Re: Reapply graceful socket shutdown on Windows (revert 29992a6a509)

From: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Nikolay Samokhvalov <nik(at)postgres(dot)ai>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: Reapply graceful socket shutdown on Windows (revert 29992a6a509)
Date: 2026-10-16 20:16:33
Message-ID: CAOYmi+nPVLfTO=whNPdyQ420GCjMg5f_BHMwTR3QeBYHWaXW=w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 8, 2026 at 1:32 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> Based on this, and the earlier discussion already in [1], I think we
> should try reverting e0f633ca6bc (the windows specific test fix) and
> 29992a6a509 (the revert of the proper fix) on master, so that we can
> watch the buildfarm and see if my analysis is correct.

Comments on the second revert (I realize you weren't the original author):

> + * However, that does not work on Windows: if the kernel closes the
> + * socket it will invoke an "abortive shutdown" that discards any data
> + * not yet sent to the client. (This is a flat-out violation of the
> + * TCP RFCs, but count on Microsoft not to care about that.)

*Does* this violate any RFCs? It seems like Windows is welcome to do
this from the point of view of TCP, annoying and underdocumented
though it may be.

> + ... When using OpenSSL, it seems that clean
> + * shutdown also requires an explicit shutdown() call.
> ...
> +#ifdef WIN32
> + shutdown(MyProcPort->sock, SD_SEND);
> + closesocket(MyProcPort->sock);
> +#endif

This still smells wrong to me. (Better, maybe.) Are we certain that
the RST we see without a shutdown(), when using OpenSSL, is the same
as the RST we see when we don't linger? Tom's last comment from the
original thread [1] is

> > > Hmm. I wonder whether using SD_BOTH behaves any differently.
>
> > With shutdown(MyProcPort->sock, SD_BOTH) the test failed for me on
> > iterations 1, 2, 3, 16 (just as without shutdown() at all).
> > So shutdown with the SD_SEND flag definitely behaves much better (I've
> > seen over 200 successful iterations).
>
> Fun. Well, I'll put in shutdown with SD_SEND for the moment,
> and we'll have to see whether we can improve further than that.
> It does sound like we may be running into OpenSSL bugs/oddities,
> not only kernel issues, so it may be impossible to do better
> on our side.

That really makes it seem like we haven't navigated the SSL_shutdown()
dance. Sending a TLS close_notify is presumably going to induce the
peer to send one back, if it hasn't already; if we immediately slam
the socket shut before reading that off the wire, then yeah, the OS is
going to reset the connection. (I say "presumably" because libpq is
using nonblocking sockets, and we don't check the result of
SSL_shutdown there, so we may not actually guarantee that the client
ever sends it. We don't check the result on the server side, either,
so if OpenSSL is trying to tell us that we're not supposed to be
calling SSL_shutdown right now, or there's some other strange problem,
we won't know.)

So to me it looks there may be multiple races/interactions going on
here. I don't want to block the patch if it gives us an empirical
improvement, but the comments may need to be adjusted, at least, while
we figure this out.

Thanks,
--Jacob

[1] https://postgr.es/m/3504774.1638901327%40sss.pgh.pa.us

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2026-10-16 20:41:27 Re: BUG #19736: `JSON_OBJECT ... ABSENT ON NULL` accepts invalid keys when the value is NULL
Previous Message Manu 2026-10-16 19:55:19 Re: [PG19] Promotion hangs behind a backend blocked on a stuck client