Re: [musl] Suggestion for thread safety

[Rich Felker <dalias@libc.org>]

On Sun, Feb 27, 2022 at 11:32:47PM +0000, Lee Shallis wrote:
> Yes, as I mentioned before, pauseCB is supposed to have it's pointer
> be changed by the developer, in other words you forgot to plugin a
> pthreads compatible call prior to your threads starting, considering
> you made that mistake I suppose it is a good thing I since switched to
> a non-redirectable pointer:
> 
> #ifdef _WIN32
> typedef DWORD WHICH;
> #else
> #include <dlfcn.h>
> #include <pthread.h>
> typedef pid_t WHICH;
> #endif
> 
> /* Which Thread Id */
> BASIC WHICH    Which();
> /* Pause thread execution to yield resources */
> BASIC void    Pause();
> 
> /* The error locks only work for conforming code, anything that doesn't will
>  * corrupt the result if it tries to do something that would need them */
> typedef struct _LOCK LOCK;
> typedef struct _GRIP GRIP;
> struct _LOCK { uint num; WHICH tid; };
> struct _GRIP { uint num; WHICH tid; GRIP *next, *prev; void *ud; };
> ....
> #ifdef _WIN32
> SHARED_EXP WHICH    Which() { return GetCurrentThreadId(); }
> SHARED_EXP void        Pause() { SwitchToThread(); }
> #else
> SHARED_EXP WHICH    Which() { return gettid(); }
> SHARED_EXP void        Pause() { pthread_yield(); }
> #endif
> ....
> SHARED_EXP void LockSiData( LOCK *shared )
> {
>     WHICH tid = Which();
>     while ( shared->tid != tid )
>     {
>         if ( !(shared->tid) )
>             shared->tid = tid;
>         Pause();
>     }
>     shared->num++;
> }
> 
> SHARED_EXP dint FreeSiData( LOCK *shared )
> {
>     if ( shared->tid == Which() )
>     {
>         shared->num--;
>         if ( !(shared->num) )
>             shared->tid = (WHICH)0;
>         return 0;
>     }
>     return EACCES;
> }
> 
> I'm not a fan of plugging in APIs directly but in this case I
> eventually decided I ought to make an exception like I did with dlopen
> etc, the name NoPause() was supposed to clue you in that to switch to
> threading you needed to change what pauseCB pointed to, I guess that
> wasn't clear enough, anyways whether you stick to the original code or
> dump my (copy-pasted) code from my library into the test and edit
> whatever you deem necessary for a valid test in your eyes, as for why
> NoPause was even necessary, it's because I'd planned on documenting
> the library to say that by default it's only single threaded mode
> compatible but with a simple change of callback from NoPause to a
> developer wrapper for the equivalent of pthread_yield() it would
> become multi-threaded safe. I switched to the object with a reference
> count because I kept making the mistake of not thinking through how I
> used it well enough causing me to eventually decide the method wasn't
> complex enough under the hood, for the malloc example I gave before it
> can be extended to support thread specific errno, all it takes is page
> locks when connecting pages together, memory locks when taking a
> section of said pages & then some #ifdef code that switches between:
> 
> a LOCK_ERRORS( errno = err; ); statement &
> a plain errno = err; statement
> 
> Either way the function can be programmed the same right up until that
> point (unless there's some way to detect in code which is suitable)

If this is supposed to have any relevance to musl, can you clarify
what that is? If not, another venue would probably be more
appropriate.

Rich