[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1328: [DRAFT] pthread_cleanup functions must be called from user space

[GitBox <gi...@apache.org>]

patacongo edited a comment on pull request #1328:
URL: https://github.com/apache/incubator-nuttx/pull/1328#issuecomment-652525167


   > But I feel like we should figure out the cancellation stuff before I do any of that.
   > 
   > > There is a complexity... pthread_exit() is also called from within the OS from task_cancelpt.c, pthread_cancel.c, pthread_create.c, sig_default.c, task_cancelpt.c, task_setcancelstate.c, task_setcanceltype.c. This may require another system call analagous to up_pthread_start().
   > 
   > I don't really understand what the analogous call would do for us there, could you expand?
   
   Well, you are making me think on my feet, but I will do the best that I can.  I might ramble a bit.
   
   First, each of the C files referenced above should be treated as a special case.  For example in pthread_create.c, pthread_exit() is only called as a error recovery measure.  In that case, there can be no pthread_cleanup routines or pthread-specific destructors registered.  So we can just replace the call to pthread_exit() with a call to nx_pthread_exit().  This might apply in other places too.
   
   But in other cases, I am sure that it will be necessary to call the user-space pthread_exit() from the kernel code in order to perform those actions in user space.  The basic problem is that in the PROTECTED build, the pthread_exit() symbol is not available to the kernel.  That is because the user blob and the kernel blob are separately built and one blob knows nothing about the symbols internal to the other.  The situation is worse in the KERNEL build because there will be multiple instances of pthread_exit() in each user block (aka, "process") and each will lie at a different virtual address -- unless of course we were to implement some kind of shared library support.
   
   The simplest way that I can think of to do that is to extend the nx_pthread_create() interface again.  On this branch it currently receives the address of the pthread_startup() function and the address of the pthread entry point.  It could be extended to also receive the address of pthread_exit() function.  This address is of no interest to the pthread_create() logic but it could be saved in pthread_tcb_s structure in the same way that the pthread_startup() function is, perhaps as 'pthread_exit';  Then, where ever pthread_exit() is called in the kernel, this could be replaced with tcb->pthread_exit().  That is not too bad.
   
   In the longer term, we need to get all of the pthread APIs out of the kernel, but this is certainly one positive step.
   
   What do you think?
   
   In KERNEL mode, there are other things to make sure we understand.  In order to call a user space function in KERNEL mode, we have to have the correct address environment in place (up_select_addrenv()).  I don't think that is will be a problem, because the main thread and all of the child pthreads share the same address environment.  Therefore, I can't think of any situation were the cancellation could occur with the wrong address environment in place.  But it is another complexity to keep in mind.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org