Skip to content
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.

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

os/kernel: pthread_mutex_* functions is called in non-pthread task #2969

Open
zhouxinhe opened this issue Apr 18, 2019 · 5 comments
Open

os/kernel: pthread_mutex_* functions is called in non-pthread task #2969

zhouxinhe opened this issue Apr 18, 2019 · 5 comments
Assignees

Comments

@zhouxinhe
Copy link
Contributor

I can find codes (invoking pthread_mutex_(un)lock() in non-pthread task) in TizenRT:
when pthread_mutex_lock() is called, pthread_mutex_add() would be executed (in case CONFIG_PTHREAD_MUTEX_UNSAFE isn't defined)...

static void pthread_mutex_add(FAR struct pthread_mutex_s *mutex)
{
	FAR struct pthread_tcb_s *rtcb = (FAR struct pthread_tcb_s *)this_task();
	irqstate_t flags;

	DEBUGASSERT(mutex->flink == NULL);

	/* Add the mutex to the list of mutexes held by this task */

	flags = irqsave();
	mutex->flink = rtcb->mhead;
	rtcb->mhead = mutex;
	irqrestore(flags);
}

in above code, it assumes that current task is a pthread task.
but it's not sure, so I try to add below assertion code in pthread_mutex_add():

DEBUGASSERT((rtcb->cmn.flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD);

then I found asstion as below:

## Starting application at 0x040C8020 ...
s5j_sflash_init: FLASH Quad Enabled
up_assert: Assertion failed at file:pthread/pthread_mutex.c line: 93 task: Idle Task

It's related to file os\drivers\wireless\scsccm_if.c (wifi is enabled).
--- Idle task is not a pthread task!
when I disabled wifi and then I got same assertion in example app code.
--- "appmain" task is not a pthread task!
I can not list all assertioins here...

My questions:
pthread_mutex_ functions should be only called in pthread task, right?

  • if yes, I suggest to add DEBUGASSERT above and then fix the assertions.
  • otherwise, there's problem:
    Any change in task management fields of struct pthread_tcb_s will damage the task management fields of real struct task_tcb_s (beause current task is non-pthread task);
    or maybe overwrite memory of next neighboring node (e.g. rtcb->mhead's offset may be larger than sizeof(struct task_tcb_s)).
@zhouxinhe
Copy link
Contributor Author

@sunghan-chang

@zhouxinhe
Copy link
Contributor Author

I checked latest code of Nuttx,
It's fixed as below:

static void pthread_mutex_add(FAR struct pthread_mutex_s *mutex)
{
  FAR struct tcb_s *rtcb = this_task();

  DEBUGASSERT(mutex->flink == NULL);

  /* Check if this is a pthread.  The main thread may also lock and unlock
   * mutexes.  The main thread, however, does not participate in the mutex
   * consistency logic.  Presumably, when the main thread exits, all of the
   * child pthreads will also terminate.
   *
   * REVISIT:  NuttX does not support that behavior at present; child pthreads
   * will persist after the main thread exits.
   */

  if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_PTHREAD)
    {
      FAR struct pthread_tcb_s *ptcb = (FAR struct pthread_tcb_s *)rtcb;
      irqstate_t flags;

      /* Add the mutex to the list of mutexes held by this pthread */

      flags        = enter_critical_section();
      mutex->flink = ptcb->mhead;
      ptcb->mhead  = mutex;
      leave_critical_section(flags);
    }
}

@xixidodo
Copy link
Contributor

if so, then it is really risky to invoke pthread_mutex_* in non-pthread task, causing undefined behavior which is hard to find the root cause.
Incorrect usage shall be fixed, and protection shall be added.

@sunghan-chang
Copy link
Contributor

@zhouxinhe @xixidodo Next week, I will check all of pthread APIs and will update them.

@sunghan-chang
Copy link
Contributor

@zhouxinhe Thank you for notice

@sunghan-chang sunghan-chang self-assigned this Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants