From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 23332 invoked from network); 26 Feb 2022 10:00:41 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 26 Feb 2022 10:00:41 -0000 Received: (qmail 30692 invoked by uid 550); 26 Feb 2022 10:00:39 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 30645 invoked from network); 26 Feb 2022 10:00:38 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:reply-to:from:date:message-id :subject:to; bh=3pn1T89ZxB9GI9xfIr3Tw7vmiTQxCLqHBrUmE7bhNgc=; b=O2mMX8h2eG2vuLsj6XqF/mTm1qUvrWGUC0CZ7fXNfm5nHlFosfhUA7htQu25xyID9E OVi9UzOJjXHEHq5WMqiObtJ2bxuUEc9LpTrRg8NGKupgsLNOLGdMQN3rJAa6VVb9XfLQ Bu0I/AUhBRzPqic9xVtjuzt/vYH/p5kMLMUIUPHjEVZQAQwiFkb3rC6P9kCAeWu2lAR8 ZIXiLcegbTuj3PpcyMauqNhZf8D+zt7ATHo1Q9AalvvQ0IcC07kjfzKt/LlMWb6fxhMQ +Kari6dPKU1a4cwV9Cp1l//ZXifLA7rDLM8pytADVNS4t+W3Kjkwa7ABZlgtOsIqA2tM v9SQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:reply-to :from:date:message-id:subject:to; bh=3pn1T89ZxB9GI9xfIr3Tw7vmiTQxCLqHBrUmE7bhNgc=; b=aL3HLY8oejurNF5M8rPYKCk9CG4XkgaDeFC6ZWL3DOMoI1a2D5c7JgEtRrZeQWml2Z sjKr9Cis35LO7o+VnryXFz7IMYB/D6SgFZtU+N6JaBSaH4SBwrKHyOnwKU58BQhg0WzZ y4icdPOyvSCoSs0WTw5FEKFuc7IT/oQsw9kNqKr6f6MQ4thhUqJW8Qy6HYNQRp5qQTZu kHtI42BeBt/ugeB0g+plxmUop9Mvhqi7mVQyJpzVO2uWafgVqWmmv1zAZMIdhq/24nGt bV8LX5JmD7JxzHJhD9MDzG4Ua+OWDz03441hB432SF7/LtK5Q0FjDWKB6JqkQIlle8bL SPhw== X-Gm-Message-State: AOAM530LTHZy9h9VycD7o3HeNi5cBu25NsF3RemPX+3qPMI9d+zdTVN+ 0IwGWQyYbpYlK7Ojwpw+r2WEH/RB3+F2AOoPRD9Zj7HD5As= X-Google-Smtp-Source: ABdhPJy2/P5HqHV53FZwP+GewXHW2NKtOIGokOy8OnAaM8CFbdBOVfCOANhfEBi+Ccwvw6sFRQgtKzrTEwU8ovLLuK8= X-Received: by 2002:a1f:9895:0:b0:331:7132:72cf with SMTP id a143-20020a1f9895000000b00331713272cfmr4970496vke.7.1645869625860; Sat, 26 Feb 2022 02:00:25 -0800 (PST) MIME-Version: 1.0 References: <20220221174223.GA2079@voyager> <20220223185746.GB2079@voyager> In-Reply-To: <20220223185746.GB2079@voyager> From: Lee Shallis Date: Sat, 26 Feb 2022 09:56:04 +0000 Message-ID: To: musl@lists.openwall.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [musl] Suggestion for thread safety On Wed, 23 Feb 2022 at 18:58, Markus Wichmann 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 #include 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,