Hi Oliver, The crash got fixed with the patch you provided before.
Do you thing the latest patch will be the right solution? Thanks, Praveen. -----Original Message----- From: Olivier Houchard [mailto:[email protected]] Sent: Friday, April 13, 2018 8:57 AM To: Willy Tarreau <[email protected]> Cc: UPPALAPATI, PRAVEEN <[email protected]>; [email protected] Subject: Re: HAProxy 1.8.X crashing Hi, On Thu, Apr 12, 2018 at 11:33:43PM +0200, Willy Tarreau wrote: > That may be the problem. But if a mux fails to initialize, then we can't > destroy them either ? Is cs_destroy() the problem here maybe ? If so, I > suspect that this approach would be more robust and would better match > the validity tests on conn->mux that are performed at many places in the > connection code : > > if (cs->conn->mux) { > cs->conn->mux->detach(cs); > } else { > conn_stop_tracking(cs->conn); > conn_xprt_close(cs->conn); > conn_free(cs->conn); > } > cs_free(cs); > > > > - one above the return SF_ERR_INTERNAL above in connect_server() to > > > handle the case where the connection already exists but not the mux. > > > > I'm not sure how this can happen, and what we should do if that happens. > > At least in the current situation we pre-create a connection just to start > to fill some information (source/destination address), without knowing the > mux yet. It's an initialization process, it may look beautiful or ugly > depending on the perspective, but in any case we must ensure that the > code overall remains reliable in this situation. And to me it looks less > ugly than picking a possibly wrong mux just to have a valid pointer > avoiding a segfault. > > But if it turns out that stabilizing the destroy path (or error path) is > not sufficient and that it breaks somewhere else, we can well adopt this > solution for the short term and see how to address it for the long term; > we already know that the outgoing connection code is severely limited to > the point of currently preventing us from using H2 on the backend, and > that's exactly why we're currently working on it. Ok, here is a patch that does exactly what you suggest. I'm not entirely happy with it, but it'll do the job, as a stopgap. I want this crash fixed :) Olivier
