Re: [musl] Suggestion for thread safety

[Lee Shallis <gb2985@gmail.com>]

On Wed, 23 Feb 2022 at 18:58, Markus Wichmann <nullplan@gmx.net> wrote:
>
> On Wed, Feb 23, 2022 at 12:30:43AM +0000, Lee Shallis wrote:
> > think of the lock as the same as a mutex, just simpler,
>
> It isn't really simpler than a fast mutex, but a lot buggier.
>
There are no bugs, I made sure to test this after all, I deliberately
created a situation where any bugs would show them selves, the only
thing that was a potential problem I've now fixed (after doing some
research to find out if 0 is ever a valid thread id), the concept
remains the same, just with support for developer calling LockSiData
twice:

#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; };

Instead of initialising with NULL, it's now just {0}, that should be a
correct way to initialise any correctly designed object, LockSiData &
FreeSiData now only need a pointer to the shared lock, the code
remains pretty much the same but instead of filling with a pointer it
fills the tid member, the num member is just for how many calls to
FreeSiData need to be made to match the calls to LockSiData, don't
call something buggy if you've never tried it, that's like being a
flat earther, talking out your ass.

> > it is supposed
> > to prevent execution races for potentially non-thread safe system
> > calls such a poorly implemented malloc (which can have it's symbol
> > overwritten by a developer implementation),
>
> Any malloc implementation has to be thread safe. It is not sensible to
> use one that isn't. It is also not sensible to pessimise the whole
> program because one day a developer might choose to do something stupid.
>
> > fprintf etc (which from
> > what I've heard are NOT thread safe)
>
> fprintf() takes a FILE and must therefore act as if flockfile() was
> called on entry and funlockfile() on exit. IOW: Accesses to the same file
> are ordered. If not, the implementation is broken.

And under my current implementation it can actually do that, it
doesn't have to care if they were called at all and just lock anyways,
the developer is already expecting a slow down because of system
calls, and since musl is a separate implementation it should address
the deficiencies of the standard implementation which in turn might
make it the default implementation on linux in the future, wouldn't
that be a grand trophy, to say your code is so good it's now the
default on a popular operating system

>
> > also errno might not be
> > thread local under some implementations,
>
> Any such implementation is fundamentally broken and cannot be repaired.
> There is no way to call a blocking system call in such a system without
> taking the lock on errno first, thereby suspending all other threads
> that might try to access errno, which is pretty much all of them, except
> maybe for some pure calculations somewhere, thereby negating any benefit
> multi-threading might have brought the program.
>
> > it's better to assume it's
> > not then to assume it is and have all hell break loose.
>
> I disagree. It is better to assume the standards are followed and fix
> problems as they occur than to assume you are programming for some kind
> of space alien computer that works by rules inconsistent with any normal
> system. Report the bug, work around it if necessary, and move on.

That last line of yours highlights the point of this code, just
because you haven't encountered the problem yet doesn't mean you
shouldn't take advantage of code that's proven to be able to work
around the problem directly should, it's like health insurance, you
don't want to need it but you'll kick yourself if you didn't have it
when you do

> I recently found myself on a system on which, unbeknownst to me,
> sendmsg() always returns 0 when called on TCP sockets. I wrote a program
> assuming it would work. It did not. I reported the bug and worked around
> the problem with malloc() and send(). That is why we test.

Another thing that highlights my point of "better to assume then to
not assume and have all hell break loose"

> > just use it to simplify any code that had to go through the
> > effort of calling pthread_mutex_create/pthread_mutex_destroy or
> > whatever,
>
> PTHREAD_MUTEX_INITIALIZER exists.

That I actually wasn't aware of, doesn't change that my initialisation
method is still simpler, I also have no need for a DestroySiData, as
long as it's free'd then it'll be in the same state it started

> > the code I gave was literally a simple
> > example of how to hide system thread safety details in pure ansi C,
>
> Nonsense. You didn't hide anything, you didn't make anything safer, and
> by staying in ANSI C, you make it impossible to achieve your goal.

Again, try the code before you talk out of your ass

> > As for your point about splitting paragraphs up, I'm not very good at
> > that as you might have noticed by now,
>
> If you don't care to be understood, I won't care to understand you. And
> it is pretty difficult to convince people that don't understand you.

So in your eyes everyone is capable of everything you can do? That's
called being ignorant, you better learn to change that because sooner
or later it's gonna bite you in the ass.

> > anyways the point of these is
> > that I wanted a simpler system than the one that is provided so that
> > if I ever put enough work into my library that it no longer needs libc
> > then I would be able to do so rather seamlessly,
>
> You might have bitten off more than you can chew with that goal. Writing
> a libc is no mean feat, and developing a library to the point it could
> replace libc takes about as much effort. Somehow people seem to think
> they'll start with memcpy() and it will stay on that level of
> complexity. It won't.

I didn't say it would be simple, also I did say "if I ever put enough
work into my library", the whole point of that line is because I knew
that in it's current state it still needs libc and I'm not heavily
motivated to change that, simply motivated enough to put some effort
towards it as I go along.

> > in other words just
> > with LOCK & pauseCB I've achieved thread safety without the file
> > knowing anything about the system api,
>
> You have indeed not done that. You have instead written the word "lock"
> enough times to give someone skim-reading the file false confidence that
> this stuff will actually work in a multi-threaded context, only to then
> fail under high load for inexplicable reasons.

Well then they should be looking at the comments too, anyways I did
after all decide to change that to just having the code directly in
file, I'm already using dlopen etc in the file, what's one more,
additionally my thread.h file includes pthreads causing the same
library to link to pthreads anyways so I might as well just include it
and directly fill the functions I need

> I keep seeing this behavior from programmers that ought to know better.
> You see, an exclusive lock consists of two parts: The mutual exclusion
> and the sleep. And yes, spinlocks skip the second part, but my point is:
> The mutual exclusion is actually the easy part, and any hack with a
> Messiah complex and a CPU manual can do it. The sleep is the hard part,
> if you want to do it right. It needs to be Goldilocks. Too short, and
> you are wasting resources (every time your thread spins in the loop is
> time the CPU could have better spent on other threads), too long and you
> are wasting time.

I see you have no idea what pthread_yield does otherwise you wouldn't
talk outta your ass like that, the whole point of the pause call is to
yield those resources, there's no getting round that part

> Your sleep is definitely too short, and you didn't even get the mutual
> exclusion part right.

You do realise that other threads will get in before that sleep ends
right? there's no way for them to take it if they entered their sleep
before the current one, and as I've stated before I created a
deliberate race scenario to test this on and it worked perfectly, you
REALLY need to start trying code before you talk about it. For
reference this is my test code:

LOCK tsidata = {0};

#ifndef _DEBUG
pthread_mutex_t debug_mutex;
#endif

dint TestPseudoMutex( THREAD *thread )
{
    ucap tid = SeekThreadTid(thread);
    ach name[bitsof(ucap)*2] = {0};

    sprintf( name, "%s %zu", __func__, tid );
    SetThreadName( thread, name );
    SetThreadDieOnExit( thread );

    while ( tsidata.tid )
        Pause();

    /* Test our pseduo mutices */
    LOCK_ERRORS( printf("%s(%zu) died\n", __func__, tid ); );

    return 0;
}

dint HelloWorld( THREAD *thread )
{
    dint i, err = 0;
    ARM *arm = SeekThreadArm(thread);
    ucap tid = SeekThreadTid( thread );
    ach name[bitsof(ucap)*2] = {0};

    sprintf( name, "%s %zu", __func__, tid );
    SetThreadName( thread, name );
    SetThreadDieOnExit( thread );

    LOCK_ERRORS( printf( "Thread %zu: Hello World!\n", tid ); );

    LockSiData( &tsidata );
    for ( i = 0; i < 3; ++i )
    {
        THREAD *child = NULL;
        LOCK_ERRORS( printf( "Spawning thread %d\n", i ); );
        err = MakeThread( thread, &child );
        if ( err )
        {
            LOCK_ERRORS
            (
                SHARED_ECHO_ERRNO( stdout, err );
                printf( "Failed to spawn thread %d\n", i );
            );
            break;
        }

        err = InitThread( thread, child, TestPseudoMutex );
        if ( err )
        {
            LOCK_ERRORS
            (
                SHARED_ECHO_ERRNO( stdout, err );
                printf( "Failed to run thread %d\n", i );
            );
            KillThread( thread, child );
            break;
        }
    }
    FreeSiData( &tsidata );

    while ( SeekBranchNumKid( arm ) )
        Pause();

    LOCK_ERRORS( printf("%s(%zu) died\n", __func__, tid ); );

    return err;
}

As you can see there's no way for the worst scenario to not be hit if
my code was buggy, it works as intended,