nusrobomaster / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

Assertion failed at file:semaphore/sem_holder.c line: 129 #6

Closed chengguizi closed 3 years ago

chengguizi commented 3 years ago
static inline FAR struct semholder_s *nxsem_allocholder(sem_t *sem)
{
  FAR struct semholder_s *pholder;

  /* Check if the "built-in" holder is being used.  We have this built-in
   * holder to optimize for the simplest case where semaphores are only
   * used to implement mutexes.
   */

#if CONFIG_SEM_PREALLOCHOLDERS > 0
  pholder = g_freeholders;
  if (pholder != NULL)
    {
      /* Remove the holder from the free list an put it into the semaphore's
       * holder list
       */

      g_freeholders    = pholder->flink;
      pholder->flink   = sem->hhead;
      sem->hhead       = pholder;

      /* Make sure the initial count is zero */

      pholder->counts  = 0;
    }
#else
  if (sem->holder[0].htcb == NULL)
    {
      pholder          = &sem->holder[0];
      pholder->counts  = 0;
    }
  else if (sem->holder[1].htcb == NULL)
    {
      pholder          = &sem->holder[1];
      pholder->counts  = 0;
    }
#endif
  else
    {
      serr("ERROR: Insufficient pre-allocated holders\n");
      pholder          = NULL;
    }

  DEBUGASSERT(pholder != NULL);
  return pholder;
}

The code failed at DEBUGASSERT(pholder != NULL);, whenever environment variable is used within the rcS script.

The issue seems to be that the assertion is overly conservative, in such a way that nullptr is not allowed to be returned. But in fact, the place where this logic is used, there is a condition check for nullptr

void nxsem_addholder_tcb(FAR struct tcb_s *htcb, FAR sem_t *sem)
{
  FAR struct semholder_s *pholder;

  /* If priority inheritance is disabled for this thread, then do not add
   * the holder.  If there are never holders of the semaphore, the priority
   * inheritance is effectively disabled.
   */

  if ((sem->flags & PRIOINHERIT_FLAGS_DISABLE) == 0)
    {
      /* Find or allocate a container for this new holder */

      pholder = nxsem_findorallocateholder(sem, htcb);
      if (pholder != NULL)
        {
          /* Then set the holder and increment the number of counts held by this
           * holder
           */

          pholder->htcb = htcb;
          pholder->counts++;
        }
    }
}

Hence, I suggest to disable CONFIG_DEBUG_ASSERTIONS in the defconfig file.

chengguizi commented 3 years ago

To verify, I have create a commit 2a5a6339d15e633b93a4563daf5a814dd622af66 , with CONFIG_DEBUG_ASSERTIONS disabled.

Alternatively, to prove my point, one can enable CONFIG_DEBUG_ASSERTIONS while ensuring that there is no set or unset command within the rcS script. Please refer to commit fb6dc1406627a74baff5cda93b6d0de821178b8f

This issue should be considered solved

chengguizi commented 3 years ago

https://nuttx.apache.org/docs/10.0.1/reference/user/05_counting_semaphore.html is a good reference to read up more about the priority inheritence and pre-allocated holders concepts.