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=-3.1 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 8780 invoked from network); 23 Feb 2022 00:35:21 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 23 Feb 2022 00:35:21 -0000 Received: (qmail 13744 invoked by uid 550); 23 Feb 2022 00:35:17 -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 13711 invoked from network); 23 Feb 2022 00:35:16 -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=HlhF7ro+bhaFFIUXzooXWyDTRrAzsZrVsFRtLo7c7CM=; b=FUEC20TfhDqnWj5F4gLtytCqXrAfQoD/FjNvk40RyGcdUJrYRTNJveBcfox7kZotoC gObkj6ZQEwATkZGIwcBTpEeSLgfKZBNXfyupBLA1MJsS5h/8/h6nht721QMj/OPWyhoM y6ZblREpz8X25eahnzSv0arO0RmKWJv9MqyMtjk08ttQFIlblozvbPrZ6YBrYTv34u/H Jp4xtjajbYOPY1M6AooGmfi7zDovGq9pHKJ8CHksd5WMTseUAWr7Y0TOnRpveZDPFmkb j0SoJumc3FUa4lPxHF9DaMOxtqTRwEV75wiwtziSQljXbMACuVle+aAScQnia//F1pbD kRfw== 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=HlhF7ro+bhaFFIUXzooXWyDTRrAzsZrVsFRtLo7c7CM=; b=ubx+Jaj0TK8UkyMZoqBM30bbqhMmOSn0sX7qhBPYLfkjnYIbRw4Crm0o780BpzXZ1M X2m8jJtm62C+ID7XKZYch1nSjN8TUxZTDUb3es+NrqQCx6SvguAJBIyDlJuW4aGevAPt pMoJ7TDQZXuy0Ekv3Eskg+4dbZ/xAkwc+DKBO8saOQCpJ+/myyocyfWFdo4nXLQOj+eV l4g7zqN4IFafa/YHTGA0ShXR3fG0/fkbc6HPkPIp8aHZAjcYrhnQmxYNKTC0Su7vh3yZ /D6i3cBC+vzoSTOVl5mvwQJrJBeW2p6CFvlXxIIZA/3NLPLFL81hOL4+TtVzlnBgICtA 2BJw== X-Gm-Message-State: AOAM533oQ43ivYNPvWE+8MJlp1mg2DvqbQ8t7sx5GrfhH0cy2Hv/f058 huaQnKlZSfPgWrgqOwDH0HwDIUwzrKMVwZ+3i7f3HAJd X-Google-Smtp-Source: ABdhPJwUP/+p4RbW/t0wS6OX6W5UAxa1cp6d+IbO2+r+o6bSZ9O0KXCWdCSEDt8ZdPDHLI3oH+G8zGnH0O0TRFAzII4= X-Received: by 2002:a67:fc13:0:b0:31c:5602:12f with SMTP id o19-20020a67fc13000000b0031c5602012fmr5117425vsq.38.1645576504343; Tue, 22 Feb 2022 16:35:04 -0800 (PST) MIME-Version: 1.0 References: <20220221174223.GA2079@voyager> In-Reply-To: <20220221174223.GA2079@voyager> From: Lee Shallis Date: Wed, 23 Feb 2022 00:30:43 +0000 Message-ID: To: musl@lists.openwall.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [musl] Suggestion for thread safety I already explained that pauseCB is supposed to be a thin wrapper to the system equiv of pthread_yield, that should tell you all you need to know about that function, I just named it pauseCB because yieldCB as name does not imply the thread will pause execution, pauseCB as a name does, anyways you're free to use pthread_yield directly as for you it IS a wrapper itself, I used a callback pointer because this is part of a custom library that decouples the thread API from thread safety, pauseCB was the ONLY callback linked to the thread API that both the header & file know of, it doesn't know anything about pthread_yield or sched_yield etc, think of the lock as the same as a mutex, just simpler, a lock only allows one thread to operate on whatever the pointer is for, in the case of LockErrors 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), fprintf etc (which from what I've heard are NOT thread safe) etc, also errno might not be thread local under some implementations, it's better to assume it's not then to assume it is and have all hell break loose. Anyways I only showed you how I used the lock system so you have a clue on how to implement your own version with this as inspiration (hence the "take what you will" statement), I'm not expecting you to do a flat copy paste, just use it to simplify any code that had to go through the effort of calling pthread_mutex_create/pthread_mutex_destroy or whatever, with this method you can just declare a local pointer in the relevant file/s and set them all to NULL before ever using them (as the stderr_lock/syserr_lock examples indicate, syserr being for things like GetLastError in windows), the code I gave was literally a simple example of how to hide system thread safety details in pure ansi C, if you want to call those APIs directly, go right ahead, I'm not stopping you. As for GRIP I wanted to simulate a semaphore just as I did with LOCK to mutex, and I wanted to do it with pointers as that was the one thing that can be guaranteed to be thread safe (assuming the developer uses the api as intended), the alternative is using thread IDs but I wasn't sure 0 would not be used for the main thread & I wasn't sure that a thread ID is always signed so I just chose to go with pointers where NULL is always understood to mean empty, anyways these where just suggestion for a simple to implement & understand mutex & semaphore substitute that can work on any system since it doesn't need to know if there's threading to operate correctly, just needs the developer to fill in pauseCB with a function that just calls the equiv of pthread_yield, nothing else, just that, the point of the call to pauseCB is to wait long enough for other writes to the same pointer to get through, even if only one gets through while waiting it's enough to fail the check after execution resumes, basically the idea is to "try to fail" instead of "try to succeed", hence why the call to pauseCB is AFTER the write, if it occured before then a data race would occur, instead we go with the assumption the data race will always occur on the pointer and just wait and see if the current thread write was the last to get through, it will be rare that many threads try to take the same pointer when it involves objects so the only thing that will potentially slow the code down a "lot" will be the system calls which one should expect to slow the code a "lot" anyways, I've actually already used the LOCK api successfully when opengl calls my debugging function I pass it AND in some custom code that is specifically designed to cause a data race where the api would show if it succeeds in emulating a mutex, I basically launched a thread to lock a pointer, then launch 3 more threads who each wait on the lock to be released before trying to print their details & exit (which is where LockErrors gets called). your point about the lack of braces around the LockErrno & LockError is duly noted though. As for your point about splitting paragraphs up, I'm not very good at that as you might have noticed by now, 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, in other words just with LOCK & pauseCB I've achieved thread safety without the file knowing anything about the system api, since I stilled ended up using dlopen/dlclose etc in the same file though I might just do away with pauseCB and just adding a Pause() function in the header for which to map the system's yield function anyways On Mon, 21 Feb 2022 at 17:42, Markus Wichmann wrote: > > On Mon, Feb 21, 2022 at 11:36:06AM +0000, Lee Shallis wrote: > > First I'll start with the code snippets I've copied from my own code: > > > > [...] > > > > #define LockErrors( ud ) LockStdErr( ud ); LockSysErr( ud ) > > #define FreeErrors( ud ) FreeSysErr( ud ); FreeStdErr( ud ) > > > > Style problem: Put a "do {} while (0)" around these, or else you can > have unintended effects. The code "if (cond) LockErrors(foo);" will > always execute "LockSysErr()". And if you put in complementary code at > the bottom of the function, it will always execute "FreeStdErr()", > leading to undefined behavior, and good luck finding it. > > > [...] > > dint err; > > LOCK_ERRORS > > ( > > errno = 0; > > *data = *data ? realloc( *data, size ) : malloc( size ); > > err = *data ? 0 : errno; > > ); > > Why lock anything here? errno is thread local, and the allocation > functions must be thread-safe. There is no need to lock anything here. > > > SHARED_EXP void LockSiData( LOCK **shared, LOCK *ptr ) > > { > > while ( *shared != ptr ) > > { > > if ( !(*shared) ) > > *shared = ptr; > > pauseCB( ptr ); > > } > > } > > > > And now I have to ask what the possible point of this is. Your call to > an external function manages to make you avoid the need for a volatile > keyword (pauseCB might access global variables, and shared might be > pointing at one, so the compiler doesn't know the state of *shared after > the return of pauseCB), however, there is probably also some kind of > barrier missing. And in practice this is going to burn up a CPU core > with completely unnecessary work. > > After you have established to your satisfaction that the lock is taken, > it would be best to suspend the current thread for exactly as long as > the lock holder takes to reach the complementary unlock function. You > can use futexes for that. Better yet, you can just use a pthread_mutex_t > for that, which does all that you need internally. > > Besides, if "shared" points to a shared variable (as the name suggests), > then this constitutes a data race (multiple threads read, some threads > write) and is therefore undefined behavior. It also has a race condition > beyond that (if one thread is suspended after seeing *shared is NULL, > but before setting *shared, another can pass the same check, set > *shared, and exit the loop. A function like pthread_yield() doesn't have > to do anything useful against that). > > It looks like you actually want some kind of compare-and-swap function > here, but again, you can also just skip it in favor of a > pthread_mutex_t. > > > [...] > > > > Take what you will of the above, the Allot function was included just > > to give you an idea of how the locks are made, as for pauseCB that > > should be re-mapped to a variant that calls something like > > pthread_yield(), it's what I tested with, anyways the general idea is > > that the shared lock defaults to NULL when not locked, when you want > > to lock it you 1st wait for it to be NULL then as soon as it's empty > > you fill it with your own pointer then wait for other threads who > > detected the same situation to fill there pointers into the shared > > pointer (assuming any other threads were trying to lock the pointer), > > when the wait is over you check again if the pointer is not matching > > your own, if not then you failed to lock and should wait longer, using > > pointers lends itself well to threading as the pointer being used to > > take the shared pointer will normally be unique to the thread (because > > rarely will anyone try to use the same pointer in multiple threads for > > this process that has a clear example in it's declaration), using > > const char * for the LOCK typedef means a thread can give more > > information about itself or the pointer it locked with if it so > > chooses. > > > > Structure your thoughts into sentences, and your sentences into > paragraphs, please. The above is one long run-on sentence, that starts > with a point about the Allot function and ends in a justification for > the type, and in between has visited many other topics. You are not > James Joyce, and you are not writing the Ulysses. > > Anyway, what I'm still trying to find is the point to all of this. > Mutual exclusion is a solved problem, and is in general hard to solve > while maintaining some semblance of performance and remaining safe. > People with more knowledge than me have poured a lot of thought into > libraries that do this, so why not use them? Especially since they are > included in the threading implementation already. > > > [GRIP is a fine-grained lock] > > Another solved problem. You can use counting semaphores to solve the > same problem in a simpler way, at least if I understood correctly. > Alternatively, you can use an interesting system of mutexes and > conditions, but that typically doesn't end up looking very pretty. > > Ciao, > Markus