On Sun, Jan 10, 2010 at 05:01:47PM +0100, Cyril Bonté wrote:
> Hi again Willy,
>
> Le Dimanche 10 Janvier 2010 00:47:14, Willy Tarreau a écrit :
> > Good catch ! Aleks and I have spent some time in the past to track
> > memory leaks in this area. This is a sensible area because it's one
> > where we're dynamically allocating memory. Obviously those two have
> > escaped us. I'm applying your patch.
>
> Maybe appsession should be forbidden in the 'defaults' section as it will not
> work in the backends.
OK, I agree since that's not allowed in the doc.
> Moreover, haproxy sergfaults when compiled with DEBUG_HASH.
I'm not sure when that feature last worked. There are several DEBUG_*
build directives that are regularly broken because they're not built
by default. Most often they're fixed when we need them :-)
> --- haproxy-1.4-dev6/src/cfgparse.c 2010-01-08 07:49:44.000000000 +0100
> +++ haproxy-1.4-dev6-appsession/src/cfgparse.c 2010-01-10 16:51:52.000000000
> +0100
> @@ -1578,6 +1578,12 @@
> else if (!strcmp(args[0], "appsession")) { /* cookie name */
> int cur_arg;
>
> + if (curproxy == &defproxy) {
> + Alert("parsing [%s:%d] : '%s' not allowed in
> 'defaults' section.\n", file, linenum, args[0]);
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> +
> if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[0],
> NULL))
> err_code |= ERR_WARN;
OK I will apply this patch. However, please check your mailer, it replaces
tabs with spaces so the patch does not apply and I have to redo it by hand
(it was the same with the last one).
> Still about appsession, I've seen in the "timeout http-keep-alive" commit
> that "timeout appsession" is supported (tested, it modifies the timeout
> defined in the previous appsession line, depending on the declaration order),
> but this appears nowhere in the documentation.
I've digged through the history and found that it was introduced after 1.3.13
with all the timeouts, then removed from the doc exactly 2 years ago after
1.3.14 (844e3ac5) but was not removed from the code. I believe I added it
when reviewing all timeouts without thinking that this one made no sense.
I'm removing that now since it's undocumented and buggy by design.
Thanks,
Willy
