Le 08/12/2017 à 05:52, Willy Tarreau a écrit :

pparissis at poseidonas in ~/repo/haproxy-1.8 on (master u=)
sudo gdb ./haproxy
GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./haproxy...done.
(gdb) run -f /etc/haproxy/haproxy-ams4-dc.cfg
Starting program: /home/pparissis/repo/haproxy-1.8/haproxy -f
/etc/haproxy/haproxy-ams4-dc.cfg
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[WARNING] 340/221611 (13628) : parsing [/etc/haproxy/haproxy-ams4-dc.cfg:103] : 
a
'http-request' rule placed after a 'use_backend' rule will still be processed 
before.
^C
Program received signal SIGINT, Interrupt.
0x000055555562e302 in register_name (name=0x5555558ee294 "selected_dc_backup",
len=18, scope=scope@entry=0x7fffffffd83c,
     alloc=alloc@entry=1, err=0x7fffffffdb28) at src/vars.c:215
215                     HA_RWLOCK_WRLOCK(VARS_LOCK, &var_names_rwlock);

Ah, this looks like a double-lock :-)
CCing Christopher who knows better than me how to track them.

Hi,

The problem is not a double-lock but an unlock on a free lock, leaving it in a bad state. This is not obvious on this trace, but compiling HAProxy with the debug on threads it is easier to observe it.

This patch is joined. It should fix the bug. Thanks Pavlos!
--
Christopher Faulet
>From 023f995f5eba5e4c18cbbde0acc2225d3846bbc7 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Fri, 8 Dec 2017 09:17:39 +0100
Subject: [PATCH] BUG/MEDIUM: threads/vars: Fix deadlock in register_name

In register_name, before locking the var_names array, we check the variable name
validity. So if we try to register an invalid or empty name, we need to return
without unlocking it (because it was never locked).

This patch must be backported in 1.8.
---
 src/vars.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/vars.c b/src/vars.c
index c8aa5ac89..566ead6ec 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -174,8 +174,7 @@ static char *register_name(const char *name, int len, enum vars_scope *scope,
 	/* Check length. */
 	if (len == 0) {
 		memprintf(err, "Empty variable name cannot be accepted");
-		res = NULL;
-		goto end;
+		return res;
 	}
 
 	/* Check scope. */
@@ -207,8 +206,7 @@ static char *register_name(const char *name, int len, enum vars_scope *scope,
 	else {
 		memprintf(err, "invalid variable name '%s'. A variable name must be start by its scope. "
 		               "The scope can be 'proc', 'sess', 'txn', 'req' or 'res'", name);
-		res = NULL;
-		goto end;
+		return res;
 	}
 
 	if (alloc)
-- 
2.13.6

Reply via email to