Skip to content

Commit 2ba7ce4

Browse files
JDevlieghereJDevlieghere
committed
[lldb] Use weak_ptr to hold on to the underlying thread plan in SBThreadPlan
Use a weak pointer to hold on to the the underlying thread plan in SBThreadPlan. When the process continues, all the popped ThreadPlans get discarded, and you can’t reuse them, so you have to create them anew. Therefore the SBThreadPlan doesn’t need to keep the ThreadPlan alive. This fixes the cleanup error in TestThreadPlanCommands.py and TestStepScripted.py caused by the thread plans never being deleted. Differential revision: https://reviews.llvm.org/D84210
1 parent fc24d1e commit 2ba7ce4

File tree

3 files changed

+67
-63
lines changed

3 files changed

+67
-63
lines changed

‎lldb/include/lldb/API/SBThreadPlan.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,11 @@ class LLDB_API SBThreadPlan {
117117
friend class lldb_private::QueueImpl;
118118
friend class SBQueueItem;
119119

120-
lldb_private::ThreadPlan *get();
120+
lldb::ThreadPlanSP GetSP() const { return m_opaque_wp.lock(); }
121+
lldb_private::ThreadPlan *get() const { return GetSP().get(); }
121122
void SetThreadPlan(const lldb::ThreadPlanSP &lldb_object_sp);
122123

123-
lldb::ThreadPlanSP m_opaque_sp;
124+
lldb::ThreadPlanWP m_opaque_wp;
124125
};
125126

126127
} // namespace lldb

‎lldb/include/lldb/lldb-forward.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ typedef std::shared_ptr<lldb_private::Thread> ThreadSP;
432432
typedef std::weak_ptr<lldb_private::Thread> ThreadWP;
433433
typedef std::shared_ptr<lldb_private::ThreadCollection> ThreadCollectionSP;
434434
typedef std::shared_ptr<lldb_private::ThreadPlan> ThreadPlanSP;
435+
typedef std::weak_ptr<lldb_private::ThreadPlan> ThreadPlanWP;
435436
typedef std::shared_ptr<lldb_private::ThreadPlanTracer> ThreadPlanTracerSP;
436437
typedef std::shared_ptr<lldb_private::TraceOptions> TraceOptionsSP;
437438
typedef std::shared_ptr<lldb_private::Type> TypeSP;

‎lldb/source/API/SBThreadPlan.cpp

Lines changed: 63 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ using namespace lldb_private;
5353
SBThreadPlan::SBThreadPlan() { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBThreadPlan); }
5454

5555
SBThreadPlan::SBThreadPlan(const ThreadPlanSP &lldb_object_sp)
56-
: m_opaque_sp(lldb_object_sp) {
56+
: m_opaque_wp(lldb_object_sp) {
5757
LLDB_RECORD_CONSTRUCTOR(SBThreadPlan, (const lldb::ThreadPlanSP &),
5858
lldb_object_sp);
5959
}
6060

6161
SBThreadPlan::SBThreadPlan(const SBThreadPlan &rhs)
62-
: m_opaque_sp(rhs.m_opaque_sp) {
62+
: m_opaque_wp(rhs.m_opaque_wp) {
6363
LLDB_RECORD_CONSTRUCTOR(SBThreadPlan, (const lldb::SBThreadPlan &), rhs);
6464
}
6565

@@ -69,8 +69,8 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name) {
6969

7070
Thread *thread = sb_thread.get();
7171
if (thread)
72-
m_opaque_sp = std::make_shared<ThreadPlanPython>(*thread, class_name,
73-
nullptr);
72+
m_opaque_wp =
73+
std::make_shared<ThreadPlanPython>(*thread, class_name, nullptr);
7474
}
7575

7676
SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name,
@@ -81,7 +81,7 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name,
8181

8282
Thread *thread = sb_thread.get();
8383
if (thread)
84-
m_opaque_sp = std::make_shared<ThreadPlanPython>(*thread, class_name,
84+
m_opaque_wp = std::make_shared<ThreadPlanPython>(*thread, class_name,
8585
args_data.m_impl_up.get());
8686
}
8787

@@ -92,28 +92,26 @@ const lldb::SBThreadPlan &SBThreadPlan::operator=(const SBThreadPlan &rhs) {
9292
SBThreadPlan, operator=,(const lldb::SBThreadPlan &), rhs);
9393

9494
if (this != &rhs)
95-
m_opaque_sp = rhs.m_opaque_sp;
95+
m_opaque_wp = rhs.m_opaque_wp;
9696
return LLDB_RECORD_RESULT(*this);
9797
}
9898
// Destructor
9999
SBThreadPlan::~SBThreadPlan() = default;
100100

101-
lldb_private::ThreadPlan *SBThreadPlan::get() { return m_opaque_sp.get(); }
102-
103101
bool SBThreadPlan::IsValid() const {
104102
LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBThreadPlan, IsValid);
105103
return this->operator bool();
106104
}
107105
SBThreadPlan::operator bool() const {
108106
LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBThreadPlan, operator bool);
109107

110-
return m_opaque_sp.get() != nullptr;
108+
return static_cast<bool>(GetSP());
111109
}
112110

113111
void SBThreadPlan::Clear() {
114112
LLDB_RECORD_METHOD_NO_ARGS(void, SBThreadPlan, Clear);
115113

116-
m_opaque_sp.reset();
114+
m_opaque_wp.reset();
117115
}
118116

119117
lldb::StopReason SBThreadPlan::GetStopReason() {
@@ -138,9 +136,10 @@ uint64_t SBThreadPlan::GetStopReasonDataAtIndex(uint32_t idx) {
138136
SBThread SBThreadPlan::GetThread() const {
139137
LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::SBThread, SBThreadPlan, GetThread);
140138

141-
if (m_opaque_sp) {
139+
ThreadPlanSP thread_plan_sp(GetSP());
140+
if (thread_plan_sp) {
142141
return LLDB_RECORD_RESULT(
143-
SBThread(m_opaque_sp->GetThread().shared_from_this()));
142+
SBThread(thread_plan_sp->GetThread().shared_from_this()));
144143
} else
145144
return LLDB_RECORD_RESULT(SBThread());
146145
}
@@ -149,50 +148,52 @@ bool SBThreadPlan::GetDescription(lldb::SBStream &description) const {
149148
LLDB_RECORD_METHOD_CONST(bool, SBThreadPlan, GetDescription,
150149
(lldb::SBStream &), description);
151150

152-
if (m_opaque_sp) {
153-
m_opaque_sp->GetDescription(description.get(), eDescriptionLevelFull);
151+
ThreadPlanSP thread_plan_sp(GetSP());
152+
if (thread_plan_sp) {
153+
thread_plan_sp->GetDescription(description.get(), eDescriptionLevelFull);
154154
} else {
155155
description.Printf("Empty SBThreadPlan");
156156
}
157157
return true;
158158
}
159159

160-
void SBThreadPlan::SetThreadPlan(const ThreadPlanSP &lldb_object_sp) {
161-
m_opaque_sp = lldb_object_sp;
160+
void SBThreadPlan::SetThreadPlan(const ThreadPlanSP &lldb_object_wp) {
161+
m_opaque_wp = lldb_object_wp;
162162
}
163163

164164
void SBThreadPlan::SetPlanComplete(bool success) {
165165
LLDB_RECORD_METHOD(void, SBThreadPlan, SetPlanComplete, (bool), success);
166166

167-
if (m_opaque_sp)
168-
m_opaque_sp->SetPlanComplete(success);
167+
ThreadPlanSP thread_plan_sp(GetSP());
168+
if (thread_plan_sp)
169+
thread_plan_sp->SetPlanComplete(success);
169170
}
170171

171172
bool SBThreadPlan::IsPlanComplete() {
172173
LLDB_RECORD_METHOD_NO_ARGS(bool, SBThreadPlan, IsPlanComplete);
173174

174-
if (m_opaque_sp)
175-
return m_opaque_sp->IsPlanComplete();
176-
else
177-
return true;
175+
ThreadPlanSP thread_plan_sp(GetSP());
176+
if (thread_plan_sp)
177+
return thread_plan_sp->IsPlanComplete();
178+
return true;
178179
}
179180

180181
bool SBThreadPlan::IsPlanStale() {
181182
LLDB_RECORD_METHOD_NO_ARGS(bool, SBThreadPlan, IsPlanStale);
182183

183-
if (m_opaque_sp)
184-
return m_opaque_sp->IsPlanStale();
185-
else
186-
return true;
184+
ThreadPlanSP thread_plan_sp(GetSP());
185+
if (thread_plan_sp)
186+
return thread_plan_sp->IsPlanStale();
187+
return true;
187188
}
188189

189190
bool SBThreadPlan::IsValid() {
190191
LLDB_RECORD_METHOD_NO_ARGS(bool, SBThreadPlan, IsValid);
191192

192-
if (m_opaque_sp)
193-
return m_opaque_sp->ValidatePlan(nullptr);
194-
else
195-
return false;
193+
ThreadPlanSP thread_plan_sp(GetSP());
194+
if (thread_plan_sp)
195+
return thread_plan_sp->ValidatePlan(nullptr);
196+
return false;
196197
}
197198

198199
// This section allows an SBThreadPlan to push another of the common types of
@@ -220,7 +221,8 @@ SBThreadPlan SBThreadPlan::QueueThreadPlanForStepOverRange(
220221
(lldb::SBAddress &, lldb::addr_t, lldb::SBError &),
221222
sb_start_address, size, error);
222223

223-
if (m_opaque_sp) {
224+
ThreadPlanSP thread_plan_sp(GetSP());
225+
if (thread_plan_sp) {
224226
Address *start_address = sb_start_address.get();
225227
if (!start_address) {
226228
return LLDB_RECORD_RESULT(SBThreadPlan());
@@ -231,19 +233,18 @@ SBThreadPlan SBThreadPlan::QueueThreadPlanForStepOverRange(
231233
start_address->CalculateSymbolContext(&sc);
232234
Status plan_status;
233235

234-
SBThreadPlan plan =
235-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepOverRange(
236+
SBThreadPlan plan = SBThreadPlan(
237+
thread_plan_sp->GetThread().QueueThreadPlanForStepOverRange(
236238
false, range, sc, eAllThreads, plan_status));
237239

238240
if (plan_status.Fail())
239241
error.SetErrorString(plan_status.AsCString());
240242
else
241-
plan.m_opaque_sp->SetPrivate(true);
242-
243+
plan.GetSP()->SetPrivate(true);
244+
243245
return LLDB_RECORD_RESULT(plan);
244-
} else {
245-
return LLDB_RECORD_RESULT(SBThreadPlan());
246246
}
247+
return LLDB_RECORD_RESULT(SBThreadPlan());
247248
}
248249

249250
SBThreadPlan
@@ -266,7 +267,8 @@ SBThreadPlan::QueueThreadPlanForStepInRange(SBAddress &sb_start_address,
266267
(lldb::SBAddress &, lldb::addr_t, lldb::SBError &),
267268
sb_start_address, size, error);
268269

269-
if (m_opaque_sp) {
270+
ThreadPlanSP thread_plan_sp(GetSP());
271+
if (thread_plan_sp) {
270272
Address *start_address = sb_start_address.get();
271273
if (!start_address) {
272274
return LLDB_RECORD_RESULT(SBThreadPlan());
@@ -278,18 +280,17 @@ SBThreadPlan::QueueThreadPlanForStepInRange(SBAddress &sb_start_address,
278280

279281
Status plan_status;
280282
SBThreadPlan plan =
281-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepInRange(
283+
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepInRange(
282284
false, range, sc, nullptr, eAllThreads, plan_status));
283285

284286
if (plan_status.Fail())
285287
error.SetErrorString(plan_status.AsCString());
286288
else
287-
plan.m_opaque_sp->SetPrivate(true);
289+
plan.GetSP()->SetPrivate(true);
288290

289291
return LLDB_RECORD_RESULT(plan);
290-
} else {
291-
return LLDB_RECORD_RESULT(SBThreadPlan());
292292
}
293+
return LLDB_RECORD_RESULT(SBThreadPlan());
293294
}
294295

295296
SBThreadPlan
@@ -312,26 +313,26 @@ SBThreadPlan::QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to,
312313
(uint32_t, bool, lldb::SBError &), frame_idx_to_step_to,
313314
first_insn, error);
314315

315-
if (m_opaque_sp) {
316+
ThreadPlanSP thread_plan_sp(GetSP());
317+
if (thread_plan_sp) {
316318
SymbolContext sc;
317-
sc = m_opaque_sp->GetThread().GetStackFrameAtIndex(0)->GetSymbolContext(
319+
sc = thread_plan_sp->GetThread().GetStackFrameAtIndex(0)->GetSymbolContext(
318320
lldb::eSymbolContextEverything);
319321

320322
Status plan_status;
321323
SBThreadPlan plan =
322-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepOut(
324+
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepOut(
323325
false, &sc, first_insn, false, eVoteYes, eVoteNoOpinion,
324326
frame_idx_to_step_to, plan_status));
325327

326328
if (plan_status.Fail())
327329
error.SetErrorString(plan_status.AsCString());
328330
else
329-
plan.m_opaque_sp->SetPrivate(true);
331+
plan.GetSP()->SetPrivate(true);
330332

331333
return LLDB_RECORD_RESULT(plan);
332-
} else {
333-
return LLDB_RECORD_RESULT(SBThreadPlan());
334334
}
335+
return LLDB_RECORD_RESULT(SBThreadPlan());
335336
}
336337

337338
SBThreadPlan
@@ -350,25 +351,25 @@ SBThreadPlan SBThreadPlan::QueueThreadPlanForRunToAddress(SBAddress sb_address,
350351
QueueThreadPlanForRunToAddress,
351352
(lldb::SBAddress, lldb::SBError &), sb_address, error);
352353

353-
if (m_opaque_sp) {
354+
ThreadPlanSP thread_plan_sp(GetSP());
355+
if (thread_plan_sp) {
354356
Address *address = sb_address.get();
355357
if (!address)
356358
return LLDB_RECORD_RESULT(SBThreadPlan());
357359

358360
Status plan_status;
359361
SBThreadPlan plan =
360-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForRunToAddress(
362+
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForRunToAddress(
361363
false, *address, false, plan_status));
362364

363365
if (plan_status.Fail())
364366
error.SetErrorString(plan_status.AsCString());
365367
else
366-
plan.m_opaque_sp->SetPrivate(true);
368+
plan.GetSP()->SetPrivate(true);
367369

368370
return LLDB_RECORD_RESULT(plan);
369-
} else {
370-
return LLDB_RECORD_RESULT(SBThreadPlan());
371371
}
372+
return LLDB_RECORD_RESULT(SBThreadPlan());
372373
}
373374

374375
SBThreadPlan
@@ -389,22 +390,22 @@ SBThreadPlan::QueueThreadPlanForStepScripted(const char *script_class_name,
389390
QueueThreadPlanForStepScripted,
390391
(const char *, lldb::SBError &), script_class_name, error);
391392

392-
if (m_opaque_sp) {
393+
ThreadPlanSP thread_plan_sp(GetSP());
394+
if (thread_plan_sp) {
393395
Status plan_status;
394396
StructuredData::ObjectSP empty_args;
395397
SBThreadPlan plan =
396-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepScripted(
398+
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepScripted(
397399
false, script_class_name, empty_args, false, plan_status));
398400

399401
if (plan_status.Fail())
400402
error.SetErrorString(plan_status.AsCString());
401403
else
402-
plan.m_opaque_sp->SetPrivate(true);
404+
plan.GetSP()->SetPrivate(true);
403405

404406
return LLDB_RECORD_RESULT(plan);
405-
} else {
406-
return LLDB_RECORD_RESULT(SBThreadPlan());
407407
}
408+
return LLDB_RECORD_RESULT(SBThreadPlan());
408409
}
409410

410411
SBThreadPlan
@@ -416,17 +417,18 @@ SBThreadPlan::QueueThreadPlanForStepScripted(const char *script_class_name,
416417
(const char *, lldb::SBStructuredData &, lldb::SBError &),
417418
script_class_name, args_data, error);
418419

419-
if (m_opaque_sp) {
420+
ThreadPlanSP thread_plan_sp(GetSP());
421+
if (thread_plan_sp) {
420422
Status plan_status;
421423
StructuredData::ObjectSP args_obj = args_data.m_impl_up->GetObjectSP();
422424
SBThreadPlan plan =
423-
SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepScripted(
425+
SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepScripted(
424426
false, script_class_name, args_obj, false, plan_status));
425427

426428
if (plan_status.Fail())
427429
error.SetErrorString(plan_status.AsCString());
428430
else
429-
plan.m_opaque_sp->SetPrivate(true);
431+
plan.GetSP()->SetPrivate(true);
430432

431433
return LLDB_RECORD_RESULT(plan);
432434
} else {

0 commit comments

Comments
 (0)