On Tue, Apr 30, 2013 at 3:27 PM, Thomas Rast <tr...@inf.ethz.ch> wrote:
> Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
>> In this particular code path, we add "base" to the delta base
>> cache. Then decide to free it, but we forgot about a dangling pointer
>> in the cache. Invalidate that entry when we free "base".
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
>> ---
>>  Some of my changes triggered a double free fault at "free(base);" in
>>  t5303. This looks like a correct thing to do, but I may be missing
>>  something (I'm not even sure how it happened). Please check.
> Can you describe how you triggered it?
> I ran all of origin/pu through valgrind tests just yesterday, and it
> found nothing (yay!), so it doesn't seem to reproduce here?

Apply this patch on top of master (no need to apply full series) and run t5303


>> @@ -2129,6 +2132,8 @@ void *unpack_entry(struct packed_git *p, off_t 
>> obj_offset,
>>                             "at offset %"PRIuMAX" from %s",
>>                             (uintmax_t)curpos, p->pack_name);
>>                       free(base);
>> +                     if (ent)
>> +                             ent->data = NULL;
>>                       data = NULL;
>>                       continue;
>>               }
> Why not clear_delta_base_cache_entry(), which also handles updating the
> lru pointers?

Simple. I didn't know about clear_de..entry().

> Also I wonder if removing free(base) is the right fix: since the failure
> is in decompressing the delta, the base might again be useful and we
> should keep it cached.

OK since you know this code much better than me, I withdraw my patch
(consider it a bug report) and let you work on a proper fix. I see you
already have the commit message ready :) Happy to test it for you if
the above instruction is still not reproducible for you.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to