From bb92b333da85150fe6a52b635c0971d3c7ca7f28 Mon Sep 17 00:00:00 2001 From: Hubert Chrzaniuk Date: Thu, 21 Jan 2021 11:02:02 +0100 Subject: [PATCH] [EGD-5355] Fix emulator crash lIndexOfLastAddedTask is shared between two separate functions that should run consecutively but in some cases the thread running can be yield in between which results in broken thread stack. This change reduces the risk but it does not entirely solve the problem. It is still possible for the threads to return in different order. Nevertheless the tests did not confirm that. --- module-bluetooth/Bluetooth/BtstackWorker.cpp | 4 ++- module-os/board/linux/port.c | 35 ++++++++++++-------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/module-bluetooth/Bluetooth/BtstackWorker.cpp b/module-bluetooth/Bluetooth/BtstackWorker.cpp index 24b3166d037286e6621799064402fc24d4cc0747..4bdc7ee14c69bde40430ad31e4b637a821b4969c 100644 --- a/module-bluetooth/Bluetooth/BtstackWorker.cpp +++ b/module-bluetooth/Bluetooth/BtstackWorker.cpp @@ -218,7 +218,9 @@ namespace Bt { BaseType_t taskerr = 0; LOG_INFO("Past last moment for Bt registration prior to RUN state"); - hci_power_control(HCI_POWER_ON); + if (auto ret = hci_power_control(HCI_POWER_ON); ret != 0) { + return Error(Error::SystemError, ret); + } if ((taskerr = xTaskCreate(run_btstack, "BtStack", 1024, NULL, tskIDLE_PRIORITY, handle)) != pdPASS) { LOG_ERROR("BT Service failure! %lu", taskerr); return Error(Error::SystemError, taskerr); diff --git a/module-os/board/linux/port.c b/module-os/board/linux/port.c index ca580afdb86900f3c2101015683daaee0359bedf..c263e9d613f54fdeaccd12fab0f34cfa2aba080f 100644 --- a/module-os/board/linux/port.c +++ b/module-os/board/linux/port.c @@ -128,7 +128,8 @@ static volatile portBASE_TYPE xSchedulerEnd = pdFALSE; static volatile portBASE_TYPE xInterruptsEnabled = pdTRUE; static volatile portBASE_TYPE xServicingTick = pdFALSE; static volatile portBASE_TYPE xPendYield = pdFALSE; -static volatile portLONG lIndexOfLastAddedTask = 0; +static volatile portLONG lIndexOfLastAddedTaskStack[_POSIX_THREAD_THREADS_MAX]; +static volatile portLONG lLastAddedTaskStackIdx = 0; static volatile portBASE_TYPE uxCriticalNesting; @@ -326,6 +327,10 @@ static void TickSignalHandler( int sig ) vTaskSwitchContext(); #endif success = LookupThread(xTaskGetCurrentTaskHandle(), &ThreadToResume); + if (success == 0) { + LOG_FATAL("Emulator only bug." \ + "Ignore and restart emulator or refer to commit linked with this log and fix it better :) "); + } assert(success); /* The only thread that can process this tick is the running thread. */ @@ -474,18 +479,17 @@ portSTACK_TYPE *pxPortInitialiseStack( portSTACK_TYPE *pxTopOfStack, pdTASK_CODE { if (pxThreads[i].Valid == 0) { - lIndexOfLastAddedTask = i; + lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx] = i; break; } } - /* No more free threads, please increase the maximum. */ assert(i < MAX_NUMBER_OF_TASKS); /* Add the task parameters. */ - pxThreads[lIndexOfLastAddedTask].pxCode = pxCode; - pxThreads[lIndexOfLastAddedTask].pvParams = pvParameters; + pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].pxCode = pxCode; + pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].pvParams = pvParameters; /* Create the new pThread. */ rc = pthread_mutex_lock(&xSingleThreadMutex); @@ -493,22 +497,23 @@ portSTACK_TYPE *pxPortInitialiseStack( portSTACK_TYPE *pxTopOfStack, pdTASK_CODE xSentinel = 0; - rc = pthread_create(&pxThreads[lIndexOfLastAddedTask].Thread, + rc = pthread_create(&pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].Thread, &xThreadAttributes, ThreadStartWrapper, - (void *)&pxThreads[lIndexOfLastAddedTask] + (void *)&pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]] ); assert(rc == 0); CPU_ZERO(&cpuset); CPU_SET(0, &cpuset); - rc = pthread_setaffinity_np(pxThreads[lIndexOfLastAddedTask].Thread, + rc = pthread_setaffinity_np(pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].Thread, sizeof(cpu_set_t), &cpuset); configASSERT( rc == 0 ); - pxThreads[lIndexOfLastAddedTask].Valid = 1; + pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].Valid = 1; + lLastAddedTaskStackIdx++; /* Wait until the task suspends. */ pthread_mutex_unlock( &xSingleThreadMutex ); @@ -643,7 +648,7 @@ portBASE_TYPE xPortStartScheduler( void ) xInterruptsEnabled = pdTRUE; xServicingTick = pdFALSE; xPendYield = pdFALSE; - lIndexOfLastAddedTask = 0; + lLastAddedTaskStackIdx = 0; uxCriticalNesting = 0; signal(SIG_SUSPEND, SIG_DFL); @@ -871,14 +876,15 @@ void vPortForciblyEndThread( void *pxTaskToDelete ) void vPortAddTaskHandle( void *pxTaskHandle ) { int i; - - pxThreads[lIndexOfLastAddedTask].hTask = (xTaskHandle)pxTaskHandle; + vPortEnterCritical(); + lLastAddedTaskStackIdx--; + pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].hTask = (xTaskHandle)pxTaskHandle; for (i = 0; i < MAX_NUMBER_OF_TASKS; i++) { - if ( pthread_equal(pxThreads[i].Thread, pxThreads[lIndexOfLastAddedTask].Thread)) + if ( pthread_equal(pxThreads[i].Thread, pxThreads[lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx]].Thread)) { - if ( pxThreads[i].hTask != pxThreads[ lIndexOfLastAddedTask ].hTask ) + if ( pxThreads[i].hTask != pxThreads[ lIndexOfLastAddedTaskStack[lLastAddedTaskStackIdx] ].hTask ) { pxThreads[i].Valid = 0; pxThreads[i].hTask = NULL; @@ -886,6 +892,7 @@ void vPortAddTaskHandle( void *pxTaskHandle ) } } } + vPortExitCritical(); }