[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] valgrind.supp: Add more valgrind suppression paths



On 07/23/2013 12:45 PM, Eric Blake wrote:
> On 07/23/2013 09:59 AM, John Ferlan wrote:
>> Update based on recent run/failures seen
>> ---
>>  tests/.valgrind.supp | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>
>> diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp
>> index 10cc3c0..f04912d 100644
>> --- a/tests/.valgrind.supp
>> +++ b/tests/.valgrind.supp
>> @@ -75,3 +75,63 @@
>>      ...
>>      obj:*/lib*/libc-2.*so*
>>  }
>> +#
>> +# commandtest validates the various threaded commands. The
>> +# virThreadCreate() routine allocates and passes args to the
>> +# new thread which now owns the 'args' and thus cannot be free'd
>> +#
>> +{
>> +    commandtestLeak1
>> +    Memcheck:Leak
>> +    fun:calloc
>> +    fun:virAlloc
>> +    fun:virThreadCreate
>> +    fun:mymain
>> +    fun:virtTestMain
> 
> Can't we call VIR_FREE(args) in the forked process?  Or is this one of
> those inconsequential leaks right before we exec in the child process?
> 

We do call VIR_FREE() in virThreadHelper() from the pthread_create().


>> +}
>> +#
>> +# The Error code requires static memory that is never free'd
>> +# for thread local storage to store error message/data
>> +#
>> +{
>> +    commandtestLeak2
>> +    Memcheck:Leak
>> +    fun:calloc
>> +    fun:virAlloc
>> +    ...
>> +    fun:vir*LastError*
>> +    fun:virEventRunDefaultImpl
>> +    fun:virCommandThreadWorker
>> +    fun:virThreadHelper
>> +    fun:start_thread
>> +    fun:clone
> 
> I thought we had the ability to wire up destructors for thread local
> storage, that gets called when the thread completes?
> 

The LastError code does wire up the destructor from how I read it.

It's very strange - running the same ranges of tests two times in a row
had different results... That is, if I did "make -C tests valgrind
TESTS=commandtest VIR_TEST_RANGE=1-4" two times in a row - one time I'd
get 1 set of tracebacks and the next time I get 2.  If I run just one
test (1-1), I get none.  I'm not sure I completely understand why


> At any rate, since all of these suppressions are solely for test files,
> we aren't masking leaks in libvirtd proper, so if it makes it easier to
> spot the introduction of new leaks, I'm okay with this patch as-is.
> 


Strange - I thought commandtestLeak3 was the most odd - only 4 tests
cause issues, but "sometimes" running them singly using VIR_TEST_RANGE
resulted in no leak shown.

I assume the failures were related to perhaps the "timing" or
"synchronization" between determining which thread was doing what and
how quickly or slowly it did it.

I can hold off on patch 3 if that's desired, too.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]