Skip to content

sys/pal/unix/sync/mutex: Fix Mutex::new() on NuttX #138400

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+16 −0
Draft

sys/pal/unix/sync/mutex: Fix Mutex::new() on NuttX #138400

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions library/std/src/sys/pal/unix/sync/mutex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,26 @@ pub struct Mutex {
}

impl Mutex {
#[cfg(not(target_os = "nuttx"))]
pub fn new() -> Mutex {
Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
}

#[cfg(target_os = "nuttx")]
pub fn new() -> Mutex {
// PTHREAD_MUTEX_INITIALIZER is highly configurable on NuttX, so it's
// hard to use a hardcoded initializer from libc.
// Instead, call pthread_mutex_init() to initialize the mutex with the
// default attributes to get the same behavior as PTHREAD_MUTEX_INITIALIZER.
let mut mutex = Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) };

unsafe {
libc::pthread_mutex_init(mutex.raw(), core::ptr::null());
Copy link
Member

Choose a reason for hiding this comment

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

NuttX allows moving a mutex after calling pthread_mutex_init? POSIX makes that UB as the mutex could eg be self-referential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in this aspect, the situation is somewhat different. When initializing a mutex, NuttX only sets the flags:

int pthread_mutex_init(FAR pthread_mutex_t *mutex,
                       FAR const pthread_mutexattr_t *attr)
{
  int status;

  sinfo("mutex=%p attr=%p\n", mutex, attr);

  if (!mutex)
    {
      return EINVAL;
    }

  /* Initialize the mutex like a semaphore with initial count = 1 */

  status = mutex_init(&mutex->mutex);
  if (status < 0)
    {
      return -status;
    }

  /* Were attributes specified?  If so, use them */

#ifdef CONFIG_PTHREAD_MUTEX_TYPES
      mutex->type  = attr ? attr->type : PTHREAD_MUTEX_DEFAULT;
#endif
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
      mutex->flink = NULL;
#  ifdef CONFIG_PTHREAD_MUTEX_BOTH
      mutex->flags = attr && attr->robust == PTHREAD_MUTEX_ROBUST ?
                     _PTHREAD_MFLAGS_ROBUST : 0;
#  else
      mutex->flags = 0;
#  endif
#endif

#if defined(CONFIG_PRIORITY_INHERITANCE) || defined(CONFIG_PRIORITY_PROTECT)
  if (attr)
    {
      status = mutex_set_protocol(&mutex->mutex, attr->proto);
      if (status < 0)
        {
          mutex_destroy(&mutex->mutex);
          return -status;
        }

#  ifdef CONFIG_PRIORITY_PROTECT
      if (attr->proto == PTHREAD_PRIO_PROTECT)
        {
          status = mutex_setprioceiling(&mutex->mutex, attr->ceiling, NULL);
          if (status < 0)
            {
              mutex_destroy(&mutex->mutex);
              return -status;
            }
        }
#  endif
    }
#endif

  return 0;
}

Copy link
Member

Choose a reason for hiding this comment

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

Is this a documented guarantee or does NuttX reserve the right to change the implementation in such a way that this becomes UB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I can't find such a document, I think I can add it to the test cases on the relevant side and incorporate it into the NuttX side to remind developers and prevent accidental changes to this behavior.

}

mutex
}

pub(super) fn raw(&self) -> *mut libc::pthread_mutex_t {
self.inner.get()
}
Expand Down
Loading