serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1712718 - /serf/trunk/buckets/allocator.c
Date Fri, 06 Nov 2015 10:22:43 GMT


> -----Original Message-----
> From: ivan@visualsvn.com [mailto:ivan@visualsvn.com] On Behalf Of Ivan
> Zhakov
> Sent: vrijdag 6 november 2015 08:12
> To: Greg Stein <gstein@gmail.com>
> Cc: dev@serf.apache.org
> Subject: Re: svn commit: r1712718 - /serf/trunk/buckets/allocator.c
> 
> On 6 November 2015 at 08:43, Greg Stein <gstein@gmail.com> wrote:
> > On Thu, Nov 5, 2015 at 9:29 AM, Ivan Zhakov <ivan@apache.org> wrote:
> >
> >> > On Thu, Nov 5, 2015 at 1:40 AM, <ivan@apache.org> wrote:
> >> >
> >> >> Author: ivan
> >> >> Date: Thu Nov  5 07:40:58 2015
> >> >> New Revision: 1712718
> >> >>
> >> >> URL: http://svn.apache.org/viewvc?rev=1712718&view=rev
> >> >> Log:
> >> >> Add another diagnostic macro SERF__DEBUG_USE_AFTER_FREE to
> detect usage
> >> of
> >> >> bucket allocator after it destroyed by pool cleanup.
> >> On 5 November 2015 at 16:31, Greg Stein <gstein@gmail.com> wrote:
> >> > Seems this should be tried straight into the allocator debugging, rather
> >> > than separately configurable. Use-after-free is quite bad, so I think it
> >> > should always die right away.
> >> >
> >> Just to confirm that I'm understanding you properly: do you suggest to
> >> enable this check by default and abort() on attempt to use bucket
> >> allocator after it's destroyed?
> >>
> >
> > It should never just abort() ...
> I think aborting on use-after-free is acceptable behavior, since we
> are pretty that program will crash anyway.
> 
> > That would be only when the debugging is
> > turned on. I'm just saying it doesn't seem useful to have *another*
> > debugging switch for USE_AFTER_FREE.
> >
> Currently we have DEBUG_DOUBLE_FREE which is *always* defined:
> [[[
> /* Define DEBUG_DOUBLE_FREE if you're interested in debugging double-
> free
>  * calls to serf_bucket_mem_free().
>  */
> #define DEBUG_DOUBLE_FREE
> ]]]
> 
> Moving #ifdef SERF__DEBUG_USE_AFTER_FREE code to #ifdef
> DEBUG_DOUBLE_FREE makes sense for me.

One major difference is that the currently enabled flag is not expensive, while enabling this
apr hash thing is very valuable when debugging... but also makes the tests run several orders
of magnitude slower.

Having three different flags though...

(Have to run in a few mins... not checking the code for which is which now)

	Bert


Mime
View raw message