> On 21 Jan 2025, at 01:40, Jacob Champion <jacob.champ...@enterprisedb.com> > wrote: > On Mon, Jan 20, 2025 at 2:10 PM Daniel Gustafsson <dan...@yesql.se> wrote:
>> + /* Done. Tear down the async implementation. */ >> + conn->cleanup_async_auth(conn); >> + conn->cleanup_async_auth = NULL; >> + Assert(conn->altsock == PGINVALID_SOCKET); >> In pqDropConnection() we set ->altsock to NULL > > (I assume you mean PGINVALID_SOCKET?) Doh, yes. >> just to be sure rather than >> assert that cleanup has done so. Shouldn't we be consistent in the >> expectation and set to NULL here as well? > > I'm not opposed; I just figured that the following code might be a bit > confusing: > > Assert(conn->altsock == PGINVALID_SOCKET); > conn->altsock = PGINVALID_SOCKET; > > But I can add a comment to the assignment to try to explain. I don't > know what the likelihood of landing code that trips that assertion is, > but an explicit assignment would at least stop problems from > cascading. It is weird, but stopping the escalation of a problem seems important. > On 21 Jan 2025, at 01:43, Jacob Champion <jacob.champ...@enterprisedb.com> > wrote: > > On second thought, I can just fail the connection if this happens. Yeah, I think that's the best option here. -- Daniel Gustafsson
