Skip to content

Commit a8bcee9

Browse files
committed
Fix incorrect freeing order in ProfiledThread (#292)
* Avoid use-after-free in ProfiledThread TLS * Improve docs for critical section (cherry picked from commit 294f19e)
1 parent 09af088 commit a8bcee9

File tree

3 files changed

+15
-2
lines changed

3 files changed

+15
-2
lines changed

ddprof-lib/src/main/cpp/criticalSection.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ CriticalSection::CriticalSection() : _entered(false), _using_fallback(false), _w
2222
int tid = OS::threadId();
2323

2424
// Hash TID to distribute across bitmap words, reducing clustering
25-
// We are OK with false colision for the fallback - it should be used only for testing when we don't have full profiler initialized
25+
// We are OK with false collision for the fallback - it should be used only for testing when we don't have full profiler initialized
2626
_word_index = hash_tid(tid) % FALLBACK_BITMAP_WORDS;
2727
uint32_t bit_index = tid % 64;
2828
_bit_mask = 1ULL << bit_index;

ddprof-lib/src/main/cpp/criticalSection.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@
2929
*
3030
* This eliminates race conditions between signal handlers and normal code
3131
* by ensuring only one can hold the critical section at a time per thread.
32+
*
33+
* !Warning! This is not a generic critical section implementation.
34+
* It relies on the fact that 'put' operations can not be preempted by the 'processing' operation.
35+
* That means that each 'put' operation will fully complete before 'processing' proceeds.
36+
*
37+
* The only preemption sequence is like this:
38+
* - processing enter
39+
* - processing acquire critical section
40+
* - signal interrupts processing; results in calling put
41+
* - put tries to acquire the critical section and fails
42+
* - put bails out
43+
* - processing proceeds and eventually releases the critical section
3244
*/
3345
class CriticalSection {
3446
private:

ddprof-lib/src/main/cpp/thread.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,9 @@ void ProfiledThread::release() {
158158
ProfiledThread *tls = (ProfiledThread *)pthread_getspecific(key);
159159
if (tls != NULL) {
160160
tls->releaseFromBuffer();
161-
delete tls;
161+
// Clear TLS pointer BEFORE deleting to prevent use-after-free if signal fires during delete
162162
pthread_setspecific(key, NULL);
163+
delete tls;
163164
}
164165
}
165166

0 commit comments

Comments
 (0)