Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOZ3c1p8A9UraG2gUuzXijgzPFtPjgMX8Yom41+Wet08Uw6ifg@mail.gmail.com>
Date: Sun, 27 Feb 2022 23:32:47 +0000
From: Lee Shallis <gb2985@...il.com>
To: musl@...ts.openwall.com
Subject: Re: Suggestion for thread safety

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)

On Sat, 26 Feb 2022 at 11:39, Joakim Sindholt <opensource@...sha.com> wrote:
>
> On Sat, 26 Feb 2022 09:56:04 +0000, Lee Shallis <gb2985@...il.com> wrote:
> > On Wed, 23 Feb 2022 at 18:58, Markus Wichmann <nullplan@....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:
>
> Did I use it wrong then?
>
> zhasha@...belwind /home/zhasha ; gcc -lpthread buglock.c
> zhasha@...belwind /home/zhasha ; ./a.out
> var = 1, expected 0
> zhasha@...belwind /home/zhasha ; ./a.out
> var = 1, expected 0
> zhasha@...belwind /home/zhasha ; ./a.out
> var = 2, expected 1
> zhasha@...belwind /home/zhasha ; ./a.out
> var = 2, expected 1
> var = 1, expected 0
> zhasha@...belwind /home/zhasha ; ./a.out
> var = 1, expected 0

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.