search

roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

Replaced pthread_mutex with pthread_key_create #420

Closed Prg1005 closed 4 years ago

Prg1005 commented 4 years ago

PR for issue #410

Using rand_seed global variable as a local storage key value, this avoids the dependency of a destructor.

pthread_key_delete is needed but I am not sure when to call it!

TravisBuddy commented 4 years ago

Build status: FAILED Build log: https://travis-ci.org/roc-streaming/roc-toolkit/builds/712353658

TravisBuddy commented 4 years ago

Build status: PASSED Build log: https://travis-ci.org/roc-streaming/roc-toolkit/builds/712368910

gavv commented 4 years ago

pthread_key_delete becomes necessary when our library is loaded using dlopen() or similar mechanism. In this case, we should remove the key when the library is unloaded. However there is nor portable way to do this, AFAIK.

I've looked into pthread_key_create implementation in glibc, and it appears that using it in a library is not that good idea. The number of keys may be limited by implementation, and the limit may be low enough to be reached in practice. It's 1024 in glibc and even lower on some BSDs.

It means two things:

  1. We can't assume that pthread_key_create() never fails. Probably the app using our library, or other libraries loaded by the app, already took all available keys. In this case it will return EAGAIN.
  2. Calling pthread_key_delete() is really important (when using dlopen).

Given all these, it seems that using POSIX TLS doesn't worth it in our simple case, and it's better to avoid it here.

We could use compiler-specific __thread or C11/C++11 thread_local, but this would require to have a separate implementation of fast_random() for older compilers, which I think also doesn't worth it.

So I think we should close this issue and PR, unfortunately.

We can leave it as is for now, or try another approach. Instead of using nrand48() + TLS, we can adopt simple atomic-based lock-free PRNG algorithm. Actually it would be even better solution, since it'll be guaranteed to be lock-free (unlike TLS) and will be fully portable (we already have portable atomics).

The implementation can be very simple. For example, in mulberry32 linked above, it's enough to replace state += 0x6D2B79F5 with an atomic add-and-fetch. Here is a real-world example of lock-free PRNG: https://github.com/liblfds/liblfds/blob/master/liblfds/liblfds7.1.0/liblfds710/inc/liblfds710/lfds710_prng.h

@Prg1005 thoughts?

gavv commented 4 years ago

I've created a new issue: #426.