Boost C++ Libraries: Ticket #2797: Two problems with thread_specific_ptr https://svn.boost.org/trac10/ticket/2797 <p> This note describes two problems with thread_specific_ptr that we have discovered and proposes solutions to both of them: </p> <p> 1) A crash at shutdown occurs when thread_specific_ptr is used from a dll and the dll gets unloaded before the thread shutdown cleanup code for thread local storage takes place. 2) A subtle leak in the implementation of thread_specific_ptr. </p> <p> Analysis of the first problem </p> <p> A thread_specific_ptr object has different thread specific “data” in different threads (this is what is returned by the get() function). The thread specific data gets destructed and removed from their thread specific store at thread shutdown (the latter is implemented as a linked list of nodes each holding a pointer to the thread specific data and a pointer to a cleanup function). However, the thread where the thread_specic_ptr object was constructed must destruct its thread specific data and remove it from the store long before thread shutdown. It must do it when the thread_specific_ptr object is destructed. The implementation in tss.hpp attempts to do this by calling reset() with a nil argument in the destructor of the thread_specific_ptr template class. </p> <p> Unfortunately, because this does not remove the entry from the linked list of thread specifics, and because it does not clear the cleanup pointer for that entry, it is possible to get a crash if the dll where the thread_specific_ptr was constructed gets unloaded before the thread shutdown cleanup code occurs. This can happen because the thread shutdown cleanup code calls the delete function of each node in the linked list even if the data in the node (the pointer to thread local data itself) is nil. The call to the delete function leads to a crash because the dll where the function pointer was defined has been unloaded. </p> <p> Proposed solution for first problem </p> <p> To avoid this issue, thread_specific_ptr should be clearing the cleanup pointer on destruction. Another solution is for thread_specific_ptr to remove the node from the linked list on destruction. </p> <p> Furthermore, in the definition of thread_specific_ptr, we should expect consistent behavior with release and reset with a nil argument. Currently, release clears the cleanup function pointer but reset with a nil argument does not. We expect they both be consistent, one way or another. </p> <p> Analysis of the second problem </p> <p> Because thread_specific_ptr does not remove the node from the tss linked list at destruction, we have a problem. The size of the linked list can become very large depending on the usage pattern of thread_specific_ptr. If the program creates and destructs thread_specific_ptr repetitively this may lead to major increase in the size of the linked list. </p> <p> A possible scenario where this can occur is if objects of class A are shared across threads, and if we keep creating and destructing them repetitively during execution. It is conceivable that the programmer need to store thread specific information in class A: </p> <p> Class A { </p> <p> private: thread_specific_ptr ts_ptr; }; </p> <p> If the program creates and destructs objects of class A repetitively this will lead to a leak as nodes get added to the cleanup list but never removed until thread shutdown. It is conceivable that the executable run out of memory because of this. </p> <p> Proposed solution for second problem: </p> <p> thread_specific_ptr must remove the node from the linked list on destruction. </p> <p> Thanks </p> <p> Habib </p> en-us Boost C++ Libraries /htdocs/site/boost.png https://svn.boost.org/trac10/ticket/2797 Trac 1.4.3 Anthony Williams Fri, 22 Oct 2010 13:25:39 GMT status, severity, milestone changed https://svn.boost.org/trac10/ticket/2797#comment:1 https://svn.boost.org/trac10/ticket/2797#comment:1 <ul> <li><strong>status</strong> <span class="trac-field-old">new</span> → <span class="trac-field-new">assigned</span> </li> <li><strong>severity</strong> <span class="trac-field-old">Showstopper</span> → <span class="trac-field-new">Problem</span> </li> <li><strong>milestone</strong> <span class="trac-field-old">Boost 1.39.0</span> → <span class="trac-field-new">To Be Determined</span> </li> </ul> <p> The main part of this issue was fixed in revision 53388 --- TSS cleanup functions are only called if there is a live value. </p> Ticket viboes Sat, 18 Aug 2012 15:41:49 GMT <link>https://svn.boost.org/trac10/ticket/2797#comment:2 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/2797#comment:2</guid> <description> <p> Didn't <a class="changeset" href="https://svn.boost.org/trac10/changeset/53389" title="Changed thread_specific_ptr to use a map for faster lookup, and erase ...">[53389]</a> fixed this issue? </p> </description> <category>Ticket</category> </item> <item> <dc:creator>viboes</dc:creator> <pubDate>Tue, 21 Aug 2012 05:02:50 GMT</pubDate> <title/> <link>https://svn.boost.org/trac10/ticket/2797#comment:3 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/2797#comment:3</guid> <description> <p> Replying to <a class="ticket" href="https://svn.boost.org/trac10/ticket/2797#comment:2" title="Comment 2">viboes</a>: </p> <blockquote class="citation"> <p> Didn't <a class="changeset" href="https://svn.boost.org/trac10/changeset/53389" title="Changed thread_specific_ptr to use a map for faster lookup, and erase ...">[53389]</a> fixed this issue? </p> </blockquote> <p> After checking the windows code, the changes in the pthread version were not applied to the windows one. I will try to apply them soon and see if this solves the issue. </p> </description> <category>Ticket</category> </item> <item> <dc:creator>viboes</dc:creator> <pubDate>Tue, 21 Aug 2012 05:37:43 GMT</pubDate> <title>owner, status changed https://svn.boost.org/trac10/ticket/2797#comment:4 https://svn.boost.org/trac10/ticket/2797#comment:4 <ul> <li><strong>owner</strong> changed from <span class="trac-author">Anthony Williams</span> to <span class="trac-author">viboes</span> </li> <li><strong>status</strong> <span class="trac-field-old">assigned</span> → <span class="trac-field-new">new</span> </li> </ul> Ticket viboes Tue, 21 Aug 2012 05:37:59 GMT status changed https://svn.boost.org/trac10/ticket/2797#comment:5 https://svn.boost.org/trac10/ticket/2797#comment:5 <ul> <li><strong>status</strong> <span class="trac-field-old">new</span> → <span class="trac-field-new">assigned</span> </li> </ul> Ticket viboes Sun, 26 Aug 2012 15:24:26 GMT milestone changed https://svn.boost.org/trac10/ticket/2797#comment:6 https://svn.boost.org/trac10/ticket/2797#comment:6 <ul> <li><strong>milestone</strong> <span class="trac-field-old">To Be Determined</span> → <span class="trac-field-new">Boost 1.52.0</span> </li> </ul> <p> Committed in trunk <a class="changeset" href="https://svn.boost.org/trac10/changeset/80236" title="Thread: 7045: make boost_thread don't depend on boost_chrono for win ...">[80236]</a>. </p> Ticket viboes Sun, 09 Sep 2012 17:43:47 GMT <link>https://svn.boost.org/trac10/ticket/2797#comment:7 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/2797#comment:7</guid> <description> <p> For your information: <a class="closed ticket" href="https://svn.boost.org/trac10/ticket/3837" title="#3837: Bugs: thread_specific_ptr returns incorrect values if another one was ... (closed: invalid)">#3837</a> has been resolved as invalid because the library is unable (respecting the expected performances) to remove the thread_specific_pointers associated to a thread when the thread exits. At least I don't know how to do it. </p> <p> The doc has been updated accordingly. Committed in trunk at revision <a class="changeset" href="https://svn.boost.org/trac10/changeset/80198" title="Thread: update tss doc to take care of #2361 and #3837">[80198]</a>. </p> </description> <category>Ticket</category> </item> <item> <dc:creator>viboes</dc:creator> <pubDate>Sun, 09 Sep 2012 18:05:36 GMT</pubDate> <title>status changed; resolution set https://svn.boost.org/trac10/ticket/2797#comment:8 https://svn.boost.org/trac10/ticket/2797#comment:8 <ul> <li><strong>status</strong> <span class="trac-field-old">assigned</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">fixed</span> </li> </ul> <p> Merged to release <a class="changeset" href="https://svn.boost.org/trac10/changeset/80450" title="Thread: merge from trunk: 1.52">[80450]</a>. </p> Ticket