On 6 November 2015 at 08:43, Greg Stein <gst...@gmail.com> wrote: > On Thu, Nov 5, 2015 at 9:29 AM, Ivan Zhakov <i...@apache.org> wrote: > >> > On Thu, Nov 5, 2015 at 1:40 AM, <i...@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 <gst...@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. >> > Your work on reporting unfreed items is a bit different, as some >> > applications *may* choose to leave some allocations until the process >> exits. >> > >> I agree, but current documentation for serf_bucket_allocator_create() >> says otherwise: >> > > Oh. Heh. I think we should leave the doc as-is for now. We may implement it > one day (tracking doesn't have to be expensive), though we'd probably not > want to abort() out of the blue for older apps that weren't careful. > -- Ivan Zhakov
