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.

Sign up for GitHub

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

Jump to bottom

implement pthread_cancel -- DO NOT MERGE, ARCHIVE ONLY. #3

Closed
wants to merge 8 commits into from
Closed

implement pthread_cancel -- DO NOT MERGE, ARCHIVE ONLY. #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
14f3912
handle pthread_join(null), implement pthread_cancel with tests
dachalco Feb 21, 2020
3c90157
uncrustify
dachalco Feb 21, 2020
9cdb932
Undo modification of worker demo thread
dachalco Feb 21, 2020
c8fd210
Enhance status messages
dachalco Feb 21, 2020
d33a7be
uncrustify
dachalco Feb 21, 2020
9dee3ce
Remove ESRCH and test code
dachalco Feb 22, 2020
d093c92
Merge branch 'master' of https://github.com/FreeRTOS/Lab-Project-Free…
dachalco Feb 22, 2020
6b1d9fb
Doc line on ESRCH no longer valid. Remove it
dachalco Feb 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions FreeRTOS-Plus-POSIX/source/FreeRTOS_POSIX_pthread.c
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,38 @@ int pthread_join( pthread_t pthread,
return iStatus;
}

/*-----------------------------------------------------------*/
int pthread_cancel( pthread_t pthread )
{
int iStatus = 0;
pthread_internal_t * pxThread = ( pthread_internal_t * ) pthread;

/* Threads may already be successfully ended elsewhere and pending full deletion in idle task. */
/* Note the the xJoinBarrier is a binary sem, so no new queue items are made once its already given */
if( pxThread && pxThread->xTaskHandle && ( eDeleted != eTaskGetState( pxThread->xTaskHandle ) ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I finished my meditation, and the implementation proposed here I believe is incorrect. I'm leaving this PR open mainly for documentation purpose. I'm commenting my reasoning below in detail.

The implementation here suspends/deletes the task without checking for "cancelability state". The detailed description of "thread cancellation" and "cancellation points" are described in POSIX Threads chapter https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_05.

Specifically:

Cancellation is controlled by the cancellation control functions. Each thread maintains its own cancelability state. Cancellation may only occur at cancellation points or when the thread is asynchronously cancelable.

(A direct counter example to the implementation in this PR is -- what if the thread being cancelled is holding a mutex? If it's detached, we can't just delete it... )

When talking about FreeRTOS+POSIX, mainly as an adapter layer to FreeRTOS APIs, we need to investigate a couple of things:

  1. How to track per thread cencelability state. (See TCB definition, and consider how to add the state in either POSIX layer or kernel layer.)
  2. Thread state machine and transit condition, see what thread cancellation means in each state. (And see how cancellation point apply in such context.)
  3. If we are talking about kernel changes, please have performance reasonings ready so that we could pick an implementation.

The quick summary is thread cancelation cannot be done with a single function as proposed in this PR.

{
if( pxThread == pthread_self() )
{
prvExitThread();
}
else
{
if( pthreadIS_JOINABLE( pxThread->xAttr.usSchedPriorityDetachState ) )
{
xSemaphoreGive( ( SemaphoreHandle_t ) &pxThread->xJoinBarrier );
vTaskSuspend( pxThread->xTaskHandle );
}
else
{
vTaskDelete( pxThread->xTaskHandle );
vPortFree( pxThread );
}
}
}

return iStatus;
}

/*-----------------------------------------------------------*/

pthread_t pthread_self( void )
Expand Down
9 changes: 9 additions & 0 deletions include/FreeRTOS_POSIX/pthread.h
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,15 @@ int pthread_barrier_init( pthread_barrier_t * barrier,
*/
int pthread_barrier_wait( pthread_barrier_t * barrier );

/**
* @brief Request cancellation of thread.
*
* @see https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cancel.html
*
* @retval 0 - Upon successful completion.
*/
int pthread_cancel( pthread_t thread );

/**
* @brief Thread creation.
*
Expand Down