> On Nov 28, 2023, at 4:29 PM, Yann Ylavic <ylavic....@gmail.com> wrote: > > On Fri, Nov 24, 2023 at 3:28 PM Ruediger Pluem <rpl...@apache.org > <mailto:rpl...@apache.org>> wrote: >> >> On 11/24/23 10:59 AM, Yann Ylavic wrote: >>> On Thu, Nov 23, 2023 at 5:47 PM Ruediger Pluem <rpl...@apache.org> wrote: >>>> >>>> On 11/23/23 5:05 PM, Yann Ylavic wrote: >>>>> On Thu, Nov 23, 2023 at 4:46 PM Yann Ylavic <ylavic....@gmail.com> wrote: >>>>>> >>>>>> On Thu, Nov 23, 2023 at 4:17 PM Ruediger Pluem <rpl...@apache.org> wrote: >>>>>>> >>>>>>> On 11/23/23 3:53 PM, Yann Ylavic wrote: >>>> >>>>>>> I am asking because I guess I am hit now by races in the byrequests LB >>>>>>> with the worker->s->lbstatus. >>>>>>> I probably want to look for a way to solve this in a more generic way >>>>>>> for various shared worker fields. >>>>>> >>>>>> lbstatus is an int so the 32bit apr_atomic API could do I think. >>>>> >>>>> I mean, if lbstatus becomes inconsistent because of the race then we >>>> >>>> My current theory is that this is the case. >>>> >>>>> can do something, but if it is e.g. that a worker switches from error >>>>> state to non-error become some threads can connect while some others >>>>> can't this is something we should address elsewhere (like a failure >>>>> threshold). >>>>> >>>>>> Maybe we need some ap_atomic_{int,long,size_t,..}_*() helpers for >>>>>> system dependent widths. It preferably would be implemented in APR but >>>>>> in the meantime we could have something in httpd already. >>>>>> What we should avoid IMO is needing 64bit atomics on 32bit systems >>>>>> (because we'd have to reimplement the mutexes etc), but apart from >>>>>> that I think we can wrap anything using the existing apr atomics. >>>>> >>>>> What we need for lbstatus is ap_atomic_int_or() and >>>>> ap_atomic_int_and_not() it seems, both could be implemented with a >>>>> cas32 loop. >>>> >>>> What functionality are ap_atomic_int_or() / ap_atomic_int_and_not() are >>>> supposed to deliver? >>>> I am having a hard time guessing it from the name :-). >>> >>> Actually I mixed ->lbstatus and ->status, so I thought we needed an >>> atomic "or" and an atomic "and not" to handle the ->status mask.. >>> >>> But ->lbstatus is much like the ->busy count and updated almost at the >>> same place, though it's an "int" and we can't reuse the same functions >>> for both :/ >>> It seems that we need to make sure that 0 <= lbstatus <= INT_MAX too, >> >> I think lbstatus is allowed to become negative. > > Tried something in https://github.com/apache/httpd/pull/396 > WDYT?
+1 for concept
