Hi Andrew, James,
On Wed, Apr 12, 2017 at 11:46:57PM +0100, Andrew Smalley wrote:
> I do not see how the old haproxy being on a separate PID could do anything
> with a socket created by a new PID.
That's what James explained. The old process tries to clean up before
leaving and tries to clean its own socket, but in practice it ends up
cleaning the new socket if the connection attempt fails due to the new
socket being saturated. That's done in destroy_uxst_socket().
> Also there is a SYN_BLOCK firewall rule required during the reload? I ask
> because we have had no reports of such a race condition.
That's totally irrelevant. In his case this is unix sockets, not TCP sockets.
James, I agree with you that what is done in destroy_uxst_socket() is
completely stupid. Even the comment saying "we might have been chrooted"
indicates that in some cases we could end up removing someone else's
socket (if permissions allow for it of course).
Given that most of the time users will run chrooted anyway, these sockets
are almost never cleaned up. So I'd vote for simply killing the contents
of destroy_uxst_socket() until we find a better idea to ensure we're really
seeing *this* process' socket. For example we could note the socket's inode
upon startup and only remove the socket if it matches this stored inode.
This is just an idea, of course.
In the mean time you can work around the problem by applying just the
one-line patch below :
static void destroy_uxst_socket(const char *path)
{
struct sockaddr_un addr;
int sock, ret;
+ return;
/* if the path was cleared, we do nothing */
if (!*path)
return;
Thanks for your detailed analysis. Would you be willing to provide a patch ?
You can reuse your original e-mail as the commit message, it's fully detailed
and explains the situation very well.
Thanks!
Willy
