[Boost-bugs] [Boost C++ Libraries] #7360: Memory leak in pthread implementation of boost::thread_specific_ptr

Subject: [Boost-bugs] [Boost C++ Libraries] #7360: Memory leak in pthread implementation of boost::thread_specific_ptr
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2012-09-10 16:44:11

#7360: Memory leak in pthread implementation of boost::thread_specific_ptr
------------------------------+---------------------------------------------
 Reporter:  cnewbold          |       Owner:         
     Type:  Bugs              |      Status:  new    
Milestone:  To Be Determined  |   Component:  None   
  Version:  Boost 1.49.0      |    Severity:  Problem
 Keywords:                    |  
------------------------------+---------------------------------------------
 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);
               }

 }}}

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/7360>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.