Skip to content

Commit 128ef9e

Browse files
jeffreytan81jeffreytan81
andauthored
Fix ASAN failure in TestSingleThreadStepTimeout.py (#102208)
This PR fixes the ASAN failure in #90930. The original PR made the assumption that parent `ThreadPlanStepOverRange`'s lifetime will always be longer than `ThreadPlanSingleThreadTimeout` leaf plan so it passes the `m_timeout_info` as reference to it. From the ASAN failure, it seems that this assumption may not be true (likely the thread stack is holding a strong reference to the leaf plan). This PR fixes this lifetime issue by using shared pointer instead of passing by reference. --------- Co-authored-by: jeffreytan81 <[email protected]>
1 parent 874890c commit 128ef9e

File tree

3 files changed

+25
-19
lines changed

3 files changed

+25
-19
lines changed

lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
namespace lldb_private {
2222

23+
class ThreadPlanSingleThreadTimeout;
2324
//
2425
// Thread plan used by single thread execution to issue timeout. This is useful
2526
// to detect potential deadlock in single thread execution. The timeout measures
@@ -46,6 +47,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
4647
bool m_isAlive = false;
4748
ThreadPlanSingleThreadTimeout::State m_last_state = State::WaitTimeout;
4849
};
50+
using TimeoutInfoSP =
51+
std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo>;
4952

5053
~ThreadPlanSingleThreadTimeout() override;
5154

@@ -54,11 +57,11 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
5457
// state. The reference of \param info is passed in so that when
5558
// ThreadPlanSingleThreadTimeout got popped its last state can be stored
5659
// in it for future resume.
57-
static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info);
60+
static void PushNewWithTimeout(Thread &thread, TimeoutInfoSP &info);
5861

5962
// Push a new ThreadPlanSingleThreadTimeout by restoring state from
6063
// input \param info and resume execution.
61-
static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info);
64+
static void ResumeFromPrevState(Thread &thread, TimeoutInfoSP &info);
6265

6366
void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
6467
bool ValidatePlan(Stream *error) override { return true; }
@@ -78,7 +81,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
7881
bool StopOthers() override;
7982

8083
private:
81-
ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info);
84+
ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfoSP &info);
8285

8386
bool IsTimeoutAsyncInterrupt(Event *event_ptr);
8487
bool HandleEvent(Event *event_ptr);
@@ -91,7 +94,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
9194
const ThreadPlanSingleThreadTimeout &
9295
operator=(const ThreadPlanSingleThreadTimeout &) = delete;
9396

94-
TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo.
97+
TimeoutInfoSP m_info; // Reference to controlling ThreadPlan's TimeoutInfo.
9598
State m_state;
9699

97100
// Lock for m_wakeup_cv and m_exit_flag between thread plan thread and timer

lldb/include/lldb/Target/TimeoutResumeAll.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ namespace lldb_private {
1919
// ResumeWithTimeout() during DoWillResume().
2020
class TimeoutResumeAll {
2121
public:
22-
TimeoutResumeAll(Thread &thread) : m_thread(thread) {}
22+
TimeoutResumeAll(Thread &thread)
23+
: m_thread(thread),
24+
m_timeout_info(
25+
std::make_shared<ThreadPlanSingleThreadTimeout::TimeoutInfo>()) {}
2326

2427
void PushNewTimeout() {
2528
ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info);
@@ -32,7 +35,7 @@ class TimeoutResumeAll {
3235

3336
private:
3437
Thread &m_thread;
35-
ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info;
38+
ThreadPlanSingleThreadTimeout::TimeoutInfoSP m_timeout_info;
3639
};
3740

3841
} // namespace lldb_private

lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@
2424
using namespace lldb_private;
2525
using namespace lldb;
2626

27-
ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread,
28-
TimeoutInfo &info)
27+
ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(
28+
Thread &thread, TimeoutInfoSP &info)
2929
: ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout",
3030
thread, eVoteNo, eVoteNoOpinion),
3131
m_info(info), m_state(State::WaitTimeout) {
3232
// TODO: reuse m_timer_thread without recreation.
3333
m_timer_thread = std::thread(TimeoutThreadFunc, this);
34-
m_info.m_isAlive = true;
35-
m_state = m_info.m_last_state;
34+
m_info->m_isAlive = true;
35+
m_state = m_info->m_last_state;
3636
}
3737

3838
ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() {
39-
m_info.m_isAlive = false;
39+
m_info->m_isAlive = false;
4040
}
4141

4242
uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMilliSeconds() {
@@ -66,7 +66,7 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) {
6666
}
6767

6868
void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
69-
TimeoutInfo &info) {
69+
TimeoutInfoSP &info) {
7070
uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout();
7171
if (timeout_in_ms == 0)
7272
return;
@@ -88,13 +88,13 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
8888
}
8989

9090
void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
91-
TimeoutInfo &info) {
91+
TimeoutInfoSP &info) {
9292
uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout();
9393
if (timeout_in_ms == 0)
9494
return;
9595

9696
// There is already an instance alive.
97-
if (info.m_isAlive)
97+
if (info->m_isAlive)
9898
return;
9999

100100
// Do not create timeout if we are not stopping other threads.
@@ -118,7 +118,7 @@ bool ThreadPlanSingleThreadTimeout::WillStop() {
118118
LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::WillStop().");
119119

120120
// Reset the state during stop.
121-
m_info.m_last_state = State::WaitTimeout;
121+
m_info->m_last_state = State::WaitTimeout;
122122
return true;
123123
}
124124

@@ -128,7 +128,7 @@ void ThreadPlanSingleThreadTimeout::DidPop() {
128128
std::lock_guard<std::mutex> lock(m_mutex);
129129
LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::DidPop().");
130130
// Tell timer thread to exit.
131-
m_info.m_isAlive = false;
131+
m_info->m_isAlive = false;
132132
}
133133
m_wakeup_cv.notify_one();
134134
// Wait for timer thread to exit.
@@ -163,12 +163,12 @@ void ThreadPlanSingleThreadTimeout::TimeoutThreadFunc(
163163
" ms",
164164
timeout_in_ms);
165165
self->m_wakeup_cv.wait_for(lock, std::chrono::milliseconds(timeout_in_ms),
166-
[self] { return !self->m_info.m_isAlive; });
166+
[self] { return !self->m_info->m_isAlive; });
167167
LLDB_LOGF(log,
168168
"ThreadPlanSingleThreadTimeout::TimeoutThreadFunc() wake up with "
169169
"m_isAlive(%d).",
170-
self->m_info.m_isAlive);
171-
if (!self->m_info.m_isAlive)
170+
self->m_info->m_isAlive);
171+
if (!self->m_info->m_isAlive)
172172
return;
173173

174174
self->HandleTimeout();

0 commit comments

Comments
 (0)