Skip to content

Commit 3551e7b

Browse files
committed
Dynamically allocate thread arrays in constructor
Previously the ThreadDebug class allocated a fixed amount of storage to track the list of active threads. This commit has the ThreadDebug constructor now take an optional parameter, indicating the maximum number of threads a developer expects to handle. It defaults to 32. I had thought about automatically growing these arrays as needed but having a debug monitor dynamically allocating memory in a crash scenario might just make a bad situation worse. I also thought about just truncating the list if it was too small but that introduced a bunch of special case handling in the code to deal with scenarios such as: * Important application threads not being suspended while halted in the debugger. * GDB not finding the halted thread in the truncated list of threads. * etc. I just left in the ASSERT which dumps a message to Serial and then enters an infinite loop if the arrays are too small to track all of the active threads.
1 parent 6878549 commit 3551e7b

File tree

2 files changed

+33
-18
lines changed

2 files changed

+33
-18
lines changed

libraries/ThreadDebug/src/ThreadDebug.cpp

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,15 @@ static volatile osThreadId_t g_haltedThreadId;
107107

108108
// The list of threads which were suspended upon entry into the debugger. These are the threads that will be resumed
109109
// upon exit from the debugger.
110-
static osThreadId_t g_suspendedThreads[MAXIMUM_ACTIVE_THREADS];
110+
static osThreadId_t* g_pSuspendedThreads;
111111
// The priorities (current and base) of each thread modified to suspend application threads when halting in debugger.
112-
static ThreadPriority g_threadPriorities[MAXIMUM_ACTIVE_THREADS];
113-
// The number of active threads that were placed in g_suspendedThreads. Some of those entries may be NULL if they were
112+
static ThreadPriority* g_pThreadPriorities;
113+
// The maximum number of threads which can be stored in the above arrays, set in ThreadDebug constructor.
114+
static uint32_t g_maxThreadCount;
115+
// The number of active threads that were placed in g_pSuspendedThreads. Some of those entries may be NULL if they were
114116
// important enough to not be suspended.
115117
static uint32_t g_threadCount;
116-
// The current index into the g_suspendedThreads array being returned from Platform_RtosGetFirstThread.
118+
// The current index into the g_pSuspendedThreads array being returned from Platform_RtosGetFirstThread.
117119
static uint32_t g_threadIndex;
118120
// Buffer to be used for storing extra thread info.
119121
static char g_threadExtraInfo[64];
@@ -210,7 +212,7 @@ static bool isDebuggerActive();
210212

211213

212214

213-
ThreadDebug::ThreadDebug(DebugCommInterface* pCommInterface, bool breakInSetup)
215+
ThreadDebug::ThreadDebug(DebugCommInterface* pCommInterface, bool breakInSetup, uint32_t maxThreadCount/* =32 */)
214216
{
215217
if (g_pComm != NULL) {
216218
// Only allow 1 ThreadDebug object to be initialized.
@@ -221,6 +223,14 @@ ThreadDebug::ThreadDebug(DebugCommInterface* pCommInterface, bool breakInSetup)
221223
g_breakInSetup = breakInSetup;
222224
g_pComm = pCommInterface;
223225

226+
// Allocate the arrays to store thread information.
227+
if (maxThreadCount < 5) {
228+
maxThreadCount = 5;
229+
}
230+
g_pSuspendedThreads = new osThreadId_t[maxThreadCount];
231+
g_pThreadPriorities = new ThreadPriority[maxThreadCount];
232+
g_maxThreadCount = maxThreadCount;
233+
224234
// Initialize the MRI core.
225235
mriInit("");
226236

@@ -281,10 +291,10 @@ static void suspendAllApplicationThreads()
281291
// the debug related threads go idle.
282292
g_idleThread = 0;
283293
g_threadCount = osThreadGetCount();
284-
ASSERT ( g_threadCount <= ARRAY_SIZE(g_suspendedThreads) );
285-
osThreadEnumerate(g_suspendedThreads, ARRAY_SIZE(g_suspendedThreads));
294+
ASSERT ( g_threadCount <= g_maxThreadCount );
295+
osThreadEnumerate(g_pSuspendedThreads, g_maxThreadCount);
286296
for (uint32_t i = 0 ; i < g_threadCount ; i++) {
287-
osThreadId_t thread = g_suspendedThreads[i];
297+
osThreadId_t thread = g_pSuspendedThreads[i];
288298
osPriority_t newPriority = osPriorityIdle;
289299
const char* pThreadName = osThreadGetName(thread);
290300
if (strcmp(pThreadName, "rtx_idle") == 0) {
@@ -293,11 +303,11 @@ static void suspendAllApplicationThreads()
293303
}
294304

295305
if (isThreadToIgnore(thread)) {
296-
g_suspendedThreads[i] = 0;
306+
g_pSuspendedThreads[i] = 0;
297307
} else {
298308
osRtxThread_t* pThread = (osRtxThread_t*)thread;
299-
g_threadPriorities[i].basePriority = pThread->priority_base;
300-
g_threadPriorities[i].priority = pThread->priority;
309+
g_pThreadPriorities[i].basePriority = pThread->priority_base;
310+
g_pThreadPriorities[i].priority = pThread->priority;
301311
osThreadSetPriority(thread, newPriority);
302312
}
303313
}
@@ -322,11 +332,11 @@ static bool isThreadToIgnore(osThreadId_t thread)
322332
static void resumeApplicationThreads()
323333
{
324334
for (uint32_t i = 0 ; i < g_threadCount ; i++) {
325-
osThreadId_t thread = g_suspendedThreads[i];
335+
osThreadId_t thread = g_pSuspendedThreads[i];
326336
if (thread != 0) {
327-
osThreadSetPriority(thread, (osPriority_t)g_threadPriorities[i].priority);
337+
osThreadSetPriority(thread, (osPriority_t)g_pThreadPriorities[i].priority);
328338
osRtxThread_t* pThread = (osRtxThread_t*)thread;
329-
pThread->priority_base = g_threadPriorities[i].basePriority;
339+
pThread->priority_base = g_pThreadPriorities[i].basePriority;
330340
}
331341
}
332342
}
@@ -422,6 +432,11 @@ static int justEnteredSetupCallback(void* pv)
422432

423433
ThreadDebug::~ThreadDebug()
424434
{
435+
delete []g_pThreadPriorities;
436+
delete []g_pSuspendedThreads;
437+
g_pThreadPriorities = NULL;
438+
g_pSuspendedThreads = NULL;
439+
425440
// IMPORTANT NOTE: You are attempting to destroy the connection to GDB which isn't allowed.
426441
// Don't allow your ThreadDebug object to go out of scope like this.
427442
debugBreak();
@@ -569,12 +584,12 @@ uint32_t Platform_RtosGetNextThreadId(void)
569584
skipNullThreadIds();
570585
if (g_threadIndex >= g_threadCount)
571586
return 0;
572-
return (uint32_t)g_suspendedThreads[g_threadIndex++];
587+
return (uint32_t)g_pSuspendedThreads[g_threadIndex++];
573588
}
574589

575590
static void skipNullThreadIds()
576591
{
577-
while (g_threadIndex < g_threadCount && isNullOrIdleThread(g_suspendedThreads[g_threadIndex]))
592+
while (g_threadIndex < g_threadCount && isNullOrIdleThread(g_pSuspendedThreads[g_threadIndex]))
578593
g_threadIndex++;
579594
}
580595

@@ -639,7 +654,7 @@ MriContext* Platform_RtosGetThreadContext(uint32_t threadId)
639654
int Platform_RtosIsThreadActive(uint32_t threadId)
640655
{
641656
for (uint32_t i = 0 ; i < g_threadCount ; i++) {
642-
if ((uint32_t)g_suspendedThreads[i] == threadId) {
657+
if ((uint32_t)g_pSuspendedThreads[i] == threadId) {
643658
return 1;
644659
}
645660
}

libraries/ThreadDebug/src/ThreadDebug.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class ThreadDebug {
5555
// -- OR --
5656
// UsbDebugCommInterface debugComm(&SerialUSB);
5757
// ThreadDebug threadDebug(&debugComm, DEBUG_BREAK_ON_SETUP);
58-
ThreadDebug(DebugCommInterface* pCommInterface, bool breakInSetup=DEBUG_BREAK_ON_SETUP);
58+
ThreadDebug(DebugCommInterface* pCommInterface, bool breakInSetup=DEBUG_BREAK_ON_SETUP, uint32_t maxThreadCount=32);
5959

6060
// Your ThreadDebug object should never go out of scope. To warn you if you do let it go out of scope by mistake,
6161
// this destructor will break into GDB and then enter an infinite loop.

0 commit comments

Comments
 (0)