[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5070: libc/semaphore:sem_init change defult protocol

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

xiaoxiang781216 edited a comment on pull request #5070:
URL: https://github.com/apache/incubator-nuttx/pull/5070#issuecomment-1000908819


   > This will also break many things for downstream projects using NuttX.
   > 
   
   Only when the project enable CONFIG_PRIORITY_INHERITANCE, otherwise no difference before and after apply this patch.
   
   > We need to make sure the wider community notices this proposed change so it can be reviewed properly. I will post on the mailing list, but there are other venues, so to everyone frequenting them, please help draw attention to this there, too.
   > 
   
   Sure.
   
   > To be clear, I am in favor of standards-compliance, provided this is in line with NuttX's [Inviolables](https://github.com/hartmannathan/incubator-nuttx/blob/master/INVIOLABLES.md):
   > 
   > * If it is standard POSIX behavior to do as this change proposes, then we should do so, but only after making sure it is reviewed properly.
   
   As far as I know, POSIX never mention the priority inheritance in spec, so it's the special behavior implemented by NuttX to improve the real time capability. But, sem_init without sem_set_protocol(SEM_NONE) activate the priority inheritance by default which make the follow common POSIX complaint program crash after the second post:
   ```
   sem_t g_sem;
   
   void *thread1_cb(void *arg)
   {
     sem_wait(&g_sem);
     return NULL;
   }
   
   void *thread2_cb(void *arg)
   {
     sleep(2);
     set_wait(&g_sem);
     return NULL;
   }
   
   void *thread3_cb(void *arg)
   {
     sleep(1);
     sem_post(&g_sem);
     sleep(2);
     sem_post(&g_sem);
     return NULL;
   }
   
   int main(int argc, char *argv[])
   {
     sem_init(&g_sem);
   
     thread1 = pthread_create(thread1_cb...);
     thread2 = pthread_create(thread2_cb...);
     thread3 = pthread_create(thread3_cb...);
   
     pthread_join(&thread1);
     pthread_join(&thread2);
     pthread_join(&thread3);
   
     sem_destroy(&g_sem);
   }
   ```
   So, it's very dangerous to enable priority inheritance without explicit indicator(e.g. a function call or an initial flag).
   
   > * If this is Linux behavior and is in contradiction to POSIX, then we should **not** do it.
   > 
   
   Of course.
   
   > Also, if this change is implemented, we need to increment the major version number. It will have to be NuttX 11.x.
   
   Sure.
   
   > As mentioned elsewhere, there are likely other parts of our tree that need review/changes.
   > 
   > Regarding whether we should do this or not, I cannot offer an opinion until I re-read POSIX details regarding semaphores to see if this is correct per our Inviolable Principles.
   
   Ok, let's wait your research.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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