Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gracefully finish the application #14

Closed
KKoovalsky opened this issue Apr 13, 2018 · 8 comments
Closed

Gracefully finish the application #14

KKoovalsky opened this issue Apr 13, 2018 · 8 comments
Assignees

Comments

@KKoovalsky
Copy link

Hey! Recently I've started using Linux port to run my application under CI system. The problem appears when I want to stop the scheduler by invoking vTaskEndScheduler. I do this in such a way:

static void testing_task(void *params);
const char testtask_name[] = "Testing";

int main()
{
	xTaskCreate(testing_task, testtask_name, 1024, NULL, 1, NULL);
	vTaskStartScheduler();
	return 0;
}

static void testing_task(void *params)
{
	run_tests();
	// Finish running the scheduler as we want only to run the tests and exit
	vTaskEndScheduler();
}

The problem is that an assertion fails:

Cleaning Up, Exiting.
intercom_plus-freertos_test: /home/user/MyProject/mdw/FreeRTOS/Source/portable/GCC/Linux/port.c:313: TickSignalHandler: Assertion `success' failed.

And valgrind says that he has found a memory leak:

==9224== 272 bytes in 1 blocks are possibly lost in loss record 5 of 9
==9224== at 0x4C2F988: calloc (vg_replace_malloc.c:711)
==9224== by 0x40138A4: allocate_dtv (dl-tls.c:322)
==9224== by 0x40138A4: _dl_allocate_tls (dl-tls.c:539)
==9224== by 0x4E4226E: allocate_stack (allocatestack.c:588)
==9224== by 0x4E4226E: pthread_create@@GLIBC_2.2.5 (pthread_create.c:539)
==9224== by 0x4082FC: pxPortInitialiseStack (port.c:466)
==9224== by 0x401943: prvInitialiseNewTask (tasks.c:1010)
==9224== by 0x40179D: xTaskCreate (tasks.c:808)
==9224== by 0x408EA9: main (tests_local_freertos.c:14)

Do I have to perform any additional steps to gracefully stop the scheduler?

@michaelbecker
Copy link
Owner

No, you shouldn't need to do anything else. This is a known issue with the Linux port (not with any of the C++ wrapper or C add-on code). I'm not sure if I introduced it when I updated William Davy's original port for FreeRTOS v5.3.0 or if it was always there. Regardless, it'a a bug and now it's on my todo list. This is probably going to be a lower priority because in a real embedded system, you almost never exit from the scheduler.

@michaelbecker michaelbecker self-assigned this Apr 14, 2018
@KKoovalsky
Copy link
Author

I've started to resolve this issue on my own but the problem is that I don't know how to gracefully finish the tasks which have been deleted using vTaskDelete.

diff --git a/mdw/FreeRTOS/Source/portable/GCC/Linux/port.c b/mdw/FreeRTOS/Source/portable/GCC/Linux/port.c
index f87fd67..1d46560 100644
--- a/mdw/FreeRTOS/Source/portable/GCC/Linux/port.c
+++ b/mdw/FreeRTOS/Source/portable/GCC/Linux/port.c
@@ -94,7 +94,6 @@
 #include "task.h"
 #include "portmacro.h"
 
-
 /* Each task maintains its own interrupt status in the critical nesting variable. */
 typedef struct ThreadState_t_
 {
@@ -117,6 +116,7 @@ static pthread_mutex_t xSuspendResumeThreadMutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t xSingleThreadMutex = PTHREAD_MUTEX_INITIALIZER;
 
 static pthread_t hMainThread;
+static pthread_t hEndSchedulerCallerThread;
 
 static volatile portBASE_TYPE xSentinel = 0;
 static volatile portBASE_TYPE xSchedulerEnd = pdFALSE;
@@ -434,9 +434,9 @@ portSTACK_TYPE *pxPortInitialiseStack( portSTACK_TYPE *pxTopOfStack, pdTASK_CODE
  
     pthread_once( &hSigSetupThread, prvSetupSignalsAndSchedulerPolicy );
 
-    /* No need to join the threads. */
+    /* Set the thread joinable if for the situation when the user wants to end scheduling. */
     pthread_attr_init( &xThreadAttributes );
-    pthread_attr_setdetachstate( &xThreadAttributes, PTHREAD_CREATE_DETACHED );
+    pthread_attr_setdetachstate( &xThreadAttributes, PTHREAD_CREATE_JOINABLE );
 
     vPortEnterCritical();
 
@@ -586,17 +586,36 @@ portBASE_TYPE xPortStartScheduler( void )
     return 0;
 }
 
-
 void vPortEndScheduler( void )
 {
     int i;
 
+	/* Ignore next or pending SIG_TICK, it mustn't execute anymore */
+    struct sigaction sigtickdeinit;
+    sigtickdeinit.sa_flags = 0;
+    sigtickdeinit.sa_handler = SIG_IGN;
+    sigfillset( &sigtickdeinit.sa_mask );
+    if ( 0 != sigaction( SIG_TICK, &sigtickdeinit, NULL ) )
+    {
+        printf( "Problem deinstalling SIG_TICK\n" );
+    }
+
     for (i = 0; i < MAX_NUMBER_OF_TASKS; i++)
     {
         if ( pxThreads[i].Valid )
-        {
+	    {
+			/* Do not cancel itself */
+			if(pthread_equal(pxThreads[i].Thread, pthread_self()))
+			{
+				pxThreads[i].Valid = 0;
+				/* Set the caller pthread_t to join this thread later */
+				hEndSchedulerCallerThread = pxThreads[i].Thread;
+				continue;
+			}
+
             /* Kill all of the threads, they are in the detached state. */
-            pthread_cancel(pxThreads[i].Thread );
+			pthread_cancel(pxThreads[i].Thread );
+			pthread_join(pxThreads[i].Thread, NULL );
             pxThreads[i].Valid = 0;
         }
     }
@@ -607,6 +626,11 @@ void vPortEndScheduler( void )
     pthread_kill( hMainThread, SIG_RESUME );
 }
 
+void vPortJoinSchedulerEndCaller()
+{
+	pthread_cancel(hEndSchedulerCallerThread);
+	pthread_join(hEndSchedulerCallerThread, NULL);
+}
 
 void vPortYieldFromISR( void )
 {

I've changed the approach, because threads are no longer detached but are left joinable for future joining.
The threads which are working when the scheduler ends finish gracefully, but the problem is that the threads which were deleted using vTaskDelete before the call to scheduler ending function are leaking because of no call to pthread_join. I can't retrieve pthread_t ID of those threads because they can be overwritten by other (new) threads, as Valid flag is reset on thread destruction. Calling pthread_join from DeleteThreadCleanupRoutine doesn't work.
@michaelbecker Have You any idea where can I join the early finished threads to avoid leaking of memory?

I've also added a function vPortJoinSchedulerEndCaller which joins the task which called vPortEndScheduler.

@vincentb1
Copy link

@KKoovalsky, Just a silly idea, couldn't we have some function vPortDeletedTasksGarbageCollector that would do this pthread_join to delete cancel threads of deleted tasks. Then the user would just have to call vPortDeletedTasksGarbageCollector form the IDLE task hook — well this means thart the user has to enable the idle task hook if not enabled, and instrument it…
Maybe the .Validfield in pxThreads[I] should be some enumerate, so make a distinction between free (=0), created (=1), and deleted_waiting_for_pthread_join (=2 for instance). You would need to replace a number of if ( pxThreads[i].Valid ) by if ( pxThreads[i].Valid == 1).

Another idea — which is not exclusive with the 1st idea above — is to do the pthead_join in the xPortStartScheduler just after the printf( "Cleaning Up, Exiting.\n" ); line of code, you would have to scan for pxThreads[i].Valid == 2 and if yes, join it.

@michaelbecker
Copy link
Owner

I've been looking at implementations of vPortEndScheduler() and confirmed that no where in any officially distributed FreeRTOS port is this actually coded. So this is all new work here.

@KKoovalsky, I looked at the code from your pull request and took the change you created where you disable the tick signal and attributed it to you in the change log in git. It was an excellent find. I also took code from vPortCleanup and added it to the end of the start scheduler call, since the code returns back there after the scheduler has stopped. I'm not taking the change to make the threads joinable. We should not need to make any pthread joinable if it's declared detached. Declaring a pthread detached allows the pthread library to automatically clean up, and it does.

I have debug code in a exitsch working branch that's incomplete. I think all memory should be reclaimed without calling vTaskDelete, and that sort of works. As of now, all memory is reclaimed except for the Timer Queue, and there's no way to solve that problem without editing the code FreeRTOS files themselves. Likewise, I added a hack in the Linux port.c file to be able to reclaim the memory allocated from the tasks TCBs themselves.

Overall, I'm not really sure where this branch is going. FreeRTOS itself doesn't really support exiting the scheduler cleanly.

@michaelbecker
Copy link
Owner

Hi All, I've been continuing to look at this, and decided this is not an issue with the Linux port of FreeRTOS. There is no way in any port, officially supported or not, to cleanly exit from FreeRTOS. To accomplish this, the kernel itself needs to be modified to clean up all of its internal structures and allocated memory. The API presented between the kernel and the lower level specific ports allows port.c code to clean up any and all data structures it allocated, but this does not include TCB, queues, etc. FreeRTOS needs to be responsible for cleaning up it's own internal data structures from vTaskEndScheduler().

I think this could be an interesting project for someone so inclined to get inside the FreeRTOS code itself and attempt to implement the cleanup code needed in vTaskEndScheduler(). But attempting to hack it from vPortEndScheduler() isn't the correct approach.

For the Linux port, I'm going to clean this up to guarantee that all of the allocations from port.c are freed. But I'm going to remove the TCB hack since it doesn't belong there. This implies that if you are using a memory checker like valgrind, it's always going to report leaked memory, which is a FreeRTOS shortcoming.

@KKoovalsky
Copy link
Author

Hi again.
I think that it's not the case, as vTaskEndScheduler cleans up ale off the resources it has allocated. The problem valgrind raises is related to pthread functionality and this is strictly related to the port so I think that it should be cleaned up in port.c.
I do not want to dig my heels in taking the approach which I proposed. I only suggest that the port should clean up its resources.
I understand that FreeRTOS is not intended to be stopped, but for the Linux port, we are losing the ability to cleanly perform tests on CI servers or development machines which could be one of the big advantages of this port.
I hope that the pull request will be merged partially - I mean the part which cancels SIG_TICK because it causes an assertion to fail sometimes. :)

@michaelbecker
Copy link
Owner

vTaskEndScheduler does not clean up the resources FreeRTOS allocates. It's not cleaning up any of the memory allocations made for TCBs. It also isn't cleaning up the associated Timer Task or the message queue it uses. These are bugs in FreeRTOS itself. It was never noticed before because no one ever calls it in an official port.
See: https://github.com/aws/amazon-freertos/blob/413f30f650170e981ce470a5789b9769a42a4445/lib/FreeRTOS/tasks.c#L2032

I agree that everyone needs to be responsible for freeing the memory they allocate. And as soon as I finish pushing the latest changes I made, port.c will do that. But port.c shouldn't be responsible for cleaning up memory that tasks.c allocates.

To fix this correctly, vTaskEndScheduler needs to free all TCB memory allocations that are still available. It also needs to clean up the timer task and it's queue. These changes need to be made in tasks.c not port.c.

@michaelbecker
Copy link
Owner

I've added all appropriate port related code and merged to master. This does not fix the FreeRTOS missing pieces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants