[rfc][icedtea-web] C++ unit-tests for memory leak w/ simple leak-detecting operator-new, along with fix

[Adam Domurad]

On 12/20/2012 10:13 AM, Pavel Tisnovsky wrote:
> Hi Adam,
>
> this patch looks very good in overall, but I'm not sure (=I'm dumb today) why can't you
> simply use ::new and ::delete operators to override them in following place:
>
> +void* operator new(size_t size) throw (std::bad_alloc) {
> +    if (!__allocations) {
> +        // This uses placement-new, which calls the constructor on a specific memory location
> +        // This is needed because we cannot call 'new' in this context, nor can we rely on static-initialization
> +        // for the set to occur before any call to 'new'!
> +        void* memory = malloc(sizeof(AllocationSet));
> +        __allocations = new (memory) AllocationSet();
> +    }
> +
> +    void* mem = malloc(size);
> +    if (mem == 0) {
> +        throw std::bad_alloc(); // ANSI/ISO compliant behavior
> +    }
> +    __allocations->insert(mem);
> +    return mem;
> +
>
> Cheers,
> Pavel

As we discussed, this would be recursive.

http://stackoverflow.com/questions/4134195/how-do-i-call-the-original-operator-new-if-i-have-overloaded-it 

indicates that the only approach is to recreate operator-new using malloc.

Thanks, will push as is (as we also discussed off-list),
-Adam

>
> ----- Adam Domurad <adomurad at redhat.com> wrote:
>> The unit test infrastructure + tests:
>> 2012-12-XX  Adam Domurad  <adomurad at redhat.com>
>>
>>       * tests/cpp-unit-tests/browser_mock.cc
>>       (mock_retainobject): New, mocks behaviour of NPAPI retainobject
>>       (mock_releaseobject): New, mocks behaviour of NPAPI releaseobject
>>       * tests/cpp-unit-tests/main.cc: Add warning of memory leak based on
>>       operator-new.
>>       * tests/cpp-unit-tests/IcedTeaScriptablePluginObjectTest.cc: New, tests
>>       for memory leak in (IcedTeaScriptableJavaObject::deAllocate) and
>>       (IcedTeaScriptableJavaPackageObject::deAllocate)
>>       * tests/cpp-unit-tests/checked_allocations.h: Defines set that does not
>>       use operator-new, to prevent recursion in overloaded operator-new
>>       * tests/cpp-unit-tests/checked_allocations.cc: Operator new overload
>>       that has allocation-set for querying live allocations.
>>
>> The fix:
>> 2012-12-XX  Adam Domurad  <adomurad at redhat.com>
>>
>>       * plugin/icedteanp/IcedTeaScriptablePluginObject.cc
>>       (IcedTeaScriptableJavaObject::deAllocate): Fix memory leak
>>       (IcedTeaScriptableJavaPackageObject::deAllocate): Fix memory leak