[Mesa-dev] [PATCH] glsl: Hook up loop_variable_state destructor to plug a memory leak.

[Kenneth Graunke]

On 06/07/2012 10:33 AM, Ian Romanick wrote:
> On 06/05/2012 04:01 PM, Kenneth Graunke wrote:
>> While ~loop_state() is already freeing the loop_variable_state objects
>> via ralloc_free(this->mem_ctx), the ~loop_variable_state() destructor
>> was never getting called, so the hash table inside loop_variable_state
>> was never getting destroyed.
>>
>> Fixes a memory leak in any shader with loops.
>>
>> NOTE: This is a candidate for stable release branches.
>>
>> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>
>> ---
>>   src/glsl/loop_analysis.h |   17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/src/glsl/loop_analysis.h b/src/glsl/loop_analysis.h
>> index 8bed1db..05c982f 100644
>> --- a/src/glsl/loop_analysis.h
>> +++ b/src/glsl/loop_analysis.h
>> @@ -140,6 +140,23 @@ public:
>>      {
>>         hash_table_dtor(this->var_hash);
>>      }
>> +
>> +   static void* operator new(size_t size, void *ctx)
>> +   {
>> +      void *lvs = ralloc_size(ctx, size);
>> +      assert(lvs != NULL);
>> +
>> +      ralloc_set_destructor(lvs, (void (*)(void*)) destructor);
>> +
>> +      return lvs;
>> +   }
>> +
>> +private:
>> +   static void
>> +   destructor(loop_variable_state *lvs)
>> +   {
>> +      lvs->~loop_variable_state();
>> +   }
>>   };
>>
>>
> 
> This just looks wrong. :)

Not disagreeing, however, see glsl_symbol_table.h.  It's the exact same
method we already use.

> Would it be better to have the real destructor
> code here and ~loop_variable_state call this->destructor?
> 
> In my dream of dreams, we'd be able to either allocate objects via
> ralloc or allocate (automatic) objects on the stack.

Yeah.