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
