POSIX Port: Remove pthread_attr_setstacksize call #1161
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On macOS, pthread_attr_setstacksize requires the stack size to be a multiple of the system page size. The previous changes here assumed the use of pthread_attr_setstack, however the transition to pthread_attr_setstacksize means we can avoid rounding the pxEndOfStack pointer. This is additionally positive because pxEndOfStack was actually being rounded in the wrong direction -- it ought to have been rounded down (_trunc) instead of up due to the stack growth direction. In my case this actually caused pxEndOfStack to end up after pxTopOfStack, which underflowed and created a huge stack size request + EXC_BAD_ACCESS in pthread_create. Signed-off-by: Paul Hollinsky <[email protected]>
|
Thank you for pointing this. Given that we are no longer using StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack,
StackType_t * pxEndOfStack,
TaskFunction_t pxCode,
void * pvParameters )
{
Thread_t * thread;
pthread_attr_t xThreadAttributes;
int iRet;
( void ) pthread_once( &hSigSetupThread, prvSetupSignalsAndSchedulerPolicy );
/*
* Store the additional thread data at the start of the stack.
*/
thread = ( Thread_t * ) ( pxTopOfStack + 1 ) - 1;
pxTopOfStack = ( StackType_t * ) thread - 1;
thread->pxCode = pxCode;
thread->pvParams = pvParameters;
thread->xDying = pdFALSE;
pthread_attr_init( &xThreadAttributes );
thread->ev = event_create();
vPortEnterCritical();
iRet = pthread_create( &thread->pthread, &xThreadAttributes,
prvWaitForStart, thread );
if( iRet != 0 )
{
prvFatalError( "pthread_create", iRet );
}
vPortExitCritical();
return pxTopOfStack;
}What do you think? |
|
@hollinsky Can you check if the above addresses the issue you are facing? |
Signed-off-by: Gaurav Aggarwal <[email protected]>
|
aggarg
changed the title
aggarg
approved these changes
Oct 24, 2024
kar-rahul-aws
approved these changes
Oct 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We have removed the use of
pthread_attr_setstackand as a result, the task stack is no longer used as the corresponding pthread's stack. There is no use of callingpthread_attr_setstacksizeas the default is always good enough and we don't need to handle OS specific cases.This PR simplifies the code by removing the call to
pthread_attr_setstacksize.Test Steps
I had a task with a 4kb stack which was causing the issue. Simply
xTaskCreatewith a 4kb stack on macOS and it will crash.Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.