Please disregard, patch 1 calls thread_isolate() with a lock held. v2 follows.
On Tue, Oct 20, 2026 at 3:04 PM David Carlier <[email protected]> wrote: > From: David Carlier <[email protected]> > > When da_haproxy_checkinst() hot-reloads the atlas, the old instance was > closed and its image freed under dadwsch_lock only, while lookups on other > threads take no lock. With several threads, a request could search a freed > image or a partially copied atlas and crash or return wrong properties. > > The new atlas is still compiled outside of any isolation, then the swap is > done between thread_isolate() and thread_release() and the old image is > freed afterwards. The lock is now only tried, so other threads keep serving > with the current atlas during the compilation instead of spinning. > > This should be backported to lower branches. > --- > addons/deviceatlas/da.c | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/addons/deviceatlas/da.c b/addons/deviceatlas/da.c > index 55429a7c5..902ceaeaf 100644 > --- a/addons/deviceatlas/da.c > +++ b/addons/deviceatlas/da.c > @@ -6,6 +6,7 @@ > #include <haproxy/api.h> > #include <haproxy/arg.h> > #include <haproxy/cfgparse.h> > +#include <haproxy/clock.h> > #include <haproxy/errors.h> > #include <haproxy/global.h> > #include <haproxy/http.h> > @@ -287,14 +288,15 @@ static void da_haproxy_checkinst(void) > > if (base[0] != 0) { > FILE *jsonp; > - void *cnew; > + void *cnew, *cold; > da_status_t status; > size_t atlassz; > char atlasp[ATLASTOKSZ] = {0}; > da_atlas_t inst; > da_property_decl_t extraprops[1] = {{NULL, 0}}; > #ifdef USE_THREAD > - HA_SPIN_LOCK(OTHER_LOCK, &dadwsch_lock); > + if (HA_SPIN_TRYLOCK(OTHER_LOCK, &dadwsch_lock) != 0) > + return; > if (base[0] == 0) { > HA_SPIN_UNLOCK(OTHER_LOCK, &dadwsch_lock); > return; > @@ -316,20 +318,22 @@ static void da_haproxy_checkinst(void) > fclose(jsonp); > if (status == DA_OK) { > if (da_atlas_open(&inst, extraprops, cnew, atlassz) == > DA_OK) { > + size_t hi, maxhdrlen = 16; > + > inst.config.cache_size = global_deviceatlas.cachesize; > + for (hi = 0; hi < inst.header_evidence_count; hi++) { > + size_t nl = > strlen(inst.header_priorities[hi].name); > + if (nl > maxhdrlen) > + maxhdrlen = nl; > + } > + thread_isolate(); > da_atlas_close(&global_deviceatlas.atlas); > - free(global_deviceatlas.atlasimgptr); > + cold = global_deviceatlas.atlasimgptr; > global_deviceatlas.atlasimgptr = cnew; > global_deviceatlas.atlas = inst; > - { > - size_t hi; > - global_deviceatlas.maxhdrlen = 16; > - for (hi = 0; hi < inst.header_evidence_count; > hi++) { > - size_t nl = > strlen(inst.header_priorities[hi].name); > - if (nl > global_deviceatlas.maxhdrlen) > - global_deviceatlas.maxhdrlen = nl; > - } > - } > + global_deviceatlas.maxhdrlen = maxhdrlen; > + thread_release(); > + free(cold); > base[0] = 0; > ha_notice("deviceatlas : new instance, data file date > `%s`.\n", > > da_getdatacreationiso8601(&global_deviceatlas.atlas)); > -- > 2.55.0 > >
