Re: [Threads-devel] Potential memory leak with thread_specific_ptr

Subject: Re: [Threads-devel] Potential memory leak with thread_specific_ptr
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-10 14:06:21

Le 10/09/12 15:39, Chris Newbold a écrit :
> Greetings. I believe I have found a potential memory leak with thread_specific_ptr. The potential leak occurs if an instance of thread_specific_ptr is created and then destroyed without ever being assigned to. Consider the following pieces of code, excerpted from the Boost.Thread library in release 1.49:
>
> >From tss.hpp:
>
>          thread_specific_ptr():
>              cleanup(detail::heap_new<delete_data>(),detail::do_heap_delete<delete_data>())
>          {}
>
>          ~thread_specific_ptr()
>          {
>              detail::set_tss_data(this,boost::shared_ptr<detail::tss_cleanup_function>(),0,true);
>          }
>
> There are only two places thread_specific_ptr calls set_tss_data: the destructor and in reset(). So if an instance of thread_specific_ptr is never used during its lifetime, the first call to set_tss_data will occur from the destructor.
>
> Now, looking at pthread/thread.cpp shows that set_tss_data has special handling for the combination of an empty cleanup function and a zero tss value which erases the corresponding node. But that handling is inside of a test for an existing node. Otherwise, a new node is unconditionally allocated. This is where I believe the leak originates.
>
> I think the tests in set_tss_data need to be rearranged in order to avoid gratuitously creating a new tss node when unused thread_specific_ptr instances are destroyed.
>
>          void set_tss_data(void const* key,
>                            boost::shared_ptr<tss_cleanup_function> func,
>                            void* tss_data,bool cleanup_existing)
>          {
>              if(tss_data_node* const current_node=find_tss_data(key))
>              {
>                  if(cleanup_existing && current_node->func && (current_node->value!=0))
>                  {
>                      (*current_node->func)(current_node->value);
>                  }
>                  if(func || (tss_data!=0))
>                  {
>                      current_node->func=func;
>                      current_node->value=tss_data;
>                  }
>                  else
>                  {
>                      erase_tss_node(key);
>                  }
>              }
>              else
>              {
>                  add_new_tss_node(key,func,tss_data);  // <<<<<< LEAK OCCURS HERE!
>              }
>          }
>
> Though the implementation of set_tss_data in win32/thread.cpp is structured somewhat differently, I believe it suffers the same problem

Hi,

thanks for catching this issue. Please could you add a ticket so that 
the issue is not lost?

Best,
Vicente