Modify

Ticket #2797 (closed Bugs: fixed)

Opened 5 years ago

Last modified 20 months ago

Two problems with thread_specific_ptr

Reported by: hkhalfal@… Owned by: viboes
Milestone: Boost 1.52.0 Component: thread
Version: Boost 1.38.0 Severity: Problem
Keywords: thread_specific_ptr; crash; leak Cc:

Description

This note describes two problems with thread_specific_ptr that we have discovered and proposes solutions to both of them:

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.

Analysis of the first problem

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.

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.

Proposed solution for first problem

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.

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.

Analysis of the second problem

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.

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:

Class A {

private: thread_specific_ptr ts_ptr; };

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.

Proposed solution for second problem:

thread_specific_ptr must remove the node from the linked list on destruction.

Thanks

Habib

Attachments

Change History

comment:1 Changed 4 years ago by anthonyw

  • Status changed from new to assigned
  • Severity changed from Showstopper to Problem
  • Milestone changed from Boost 1.39.0 to To Be Determined

The main part of this issue was fixed in revision 53388 --- TSS cleanup functions are only called if there is a live value.

comment:2 follow-up: ↓ 3 Changed 20 months ago by viboes

Didn't [53389] fixed this issue?

comment:3 in reply to: ↑ 2 Changed 20 months ago by viboes

Replying to viboes:

Didn't [53389] fixed this issue?

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.

comment:4 Changed 20 months ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from assigned to new

comment:5 Changed 20 months ago by viboes

  • Status changed from new to assigned

comment:6 Changed 20 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.52.0

Committed in trunk [80236].

comment:7 Changed 20 months ago by viboes

For your information: #3837 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.

The doc has been updated accordingly. Committed in trunk at revision [80198].

comment:8 Changed 20 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

Merged to release [80450].

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.