Memory leak in pthread implementation of boost::thread_specific_ptr

[Chris Newbold]

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! 
            } 
        }

Re-writing the last part of set_tss_data as follows appears to address the leaks we've observed:

***************
*** 595,601 ****
                      erase_tss_node(key);
                  }
              }
!             else
              {
                  add_new_tss_node(key,func,tss_data);
              }
--- 595,601 ----
                      erase_tss_node(key);
                  }
              }
!             else if(func || (tss_data!=0))
              {
                  add_new_tss_node(key,func,tss_data);
              }

[viboes]

Committed in trunk revision 80496.(posix part)

[viboes]

Committed in trunk revision [80533].(windows part)

[viboes]

Committed revision [80668].