pthread_once: g_lock may lead deadlock

[hujun260]

Summary

For programs with the dependencies logic in pthread_once callback , using global locks may cause deadlock:

task A
pthread_once()
|
|-> nxrmutex_lock(&g_lock);
 -> init_routine(); // callback to wait task B
                                                  task B
                                                  pthread_once()
                                                  |
                                                   ->nxrmutex_lock(&g_lock); // Deadlock
                                                   ->init_routine(); // hold resource to wake up task A

Impact

pthread_once

Testing

internal

[jerpelea]

please add commit message describing the change

[pkarashchenko]

Sorry, reviewing it from mobile. Is this a recursive mutex?

[hujun260]

rmutex is not necessary. If a user uses the same once_control in init_routine, it can cause a deadlock, which we believe is a user code design issue.

If rmutex is used, it can cause stack overflow and make it difficult to troubleshoot

[pkarashchenko]

Why rmutex can cause stack overflow?

[hujun260]

Why rmutex can cause stack overflow?

Too many levels of recursive calls

[pkarashchenko]

I think taking into account once state value I do not expect more than one level of nesting.

[hujun260]

I misunderstood. It should not cause recursive calls, but using rmutex cannot guarantee that init_route execution is complete when pthread_once returns

[pkarashchenko]

The POSIX specifies:

If init_routine recursively calls pthread_once() with the same once_control, the recursive call will not call the specified init_routine, and thus the specified init_routine will not complete, and thus the recursive call to pthread_once() will not return.

Not sure how that should be treated. Especially the last sentence the recursive call to pthread_once() will not return.

[pkarashchenko]

Also POSIX contains:

For dynamic library initialization in a multi-threaded process, if an initialization flag is used the flag needs to be protected against modification by multiple threads simultaneously calling into the library. This can be done by using a mutex (initialized by assigning PTHREAD_MUTEX_INITIALIZER). However, the better solution is to use pthread_once() which is designed for exactly this purpose

So in this case I do not see a difference from PTHREAD_MUTEX_INITIALIZER described case in POSIX

[xiaoxiang781216]

It's normally a program error to call an initialization routine recursively, it's good to catch this error through the non-recursive mutex(dead lock).

[xiaoxiang781216]

@pkarashchenko do you have more question?

[pkarashchenko]

Also POSIX contains:

For dynamic library initialization in a multi-threaded process, if an initialization flag is used the flag needs to be protected against modification by multiple threads simultaneously calling into the library. This can be done by using a mutex (initialized by assigning PTHREAD_MUTEX_INITIALIZER). However, the better solution is to use pthread_once() which is designed for exactly this purpose

So in this case I do not see a difference from PTHREAD_MUTEX_INITIALIZER described case in POSIX