From e5ec7bf15455f45fb08623436d2e21e2b4ea1619 Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 14 Aug 2024 13:08:20 +0800 Subject: [PATCH] Change server.daylight_active to an atomic variable (#876) We are updating this variable in the main thread, and the child threads can printing the logs at the same time. This generating a warning in SANITIZER=thread: ``` WARNING: ThreadSanitizer: data race (pid=74208) Read of size 4 at 0x000102875c10 by thread T3: #0 serverLogRaw :52173615 (valkey-server:x86_64+0x10003c556) #1 _serverLog :52173615 (valkey-server:x86_64+0x10003ca89) #2 bioProcessBackgroundJobs :52173615 (valkey-server:x86_64+0x1001402c9) Previous write of size 4 at 0x000102875c10 by main thread (mutexes: write M0): #0 afterSleep :52173615 (valkey-server:x86_64+0x10004989b) #1 aeProcessEvents :52173615 (valkey-server:x86_64+0x100031e52) #2 main :52173615 (valkey-server:x86_64+0x100064a3c) #3 start :52173615 (dyld:x86_64+0xfffffffffff5c365) #4 start :52173615 (dyld:x86_64+0xfffffffffff5c365) ``` The refresh of daylight_active is not real time, we update it in aftersleep, so we don't need a strong synchronization, so using memory_order_relaxed. But also noted we are doing load/store operations only for daylight_active, which is an aligned 32-bit integer, so using memory_order_relaxed will not provide more consistency than what we have today. So this is just a cleanup that to clear the warning. Signed-off-by: Binbin --- src/server.c | 5 +++-- src/server.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/server.c b/src/server.c index 6979d0981d..2316afb7bf 100644 --- a/src/server.c +++ b/src/server.c @@ -131,10 +131,11 @@ void serverLogRaw(int level, const char *msg) { struct timeval tv; int role_char; pid_t pid = getpid(); + int daylight_active = atomic_load_explicit(&server.daylight_active, memory_order_relaxed); gettimeofday(&tv, NULL); struct tm tm; - nolocks_localtime(&tm, tv.tv_sec, server.timezone, server.daylight_active); + nolocks_localtime(&tm, tv.tv_sec, server.timezone, daylight_active); off = strftime(buf, sizeof(buf), "%d %b %Y %H:%M:%S.", &tm); snprintf(buf + off, sizeof(buf) - off, "%03d", (int)tv.tv_usec / 1000); if (server.sentinel_mode) { @@ -1091,7 +1092,7 @@ static inline void updateCachedTimeWithUs(int update_daylight_info, const long l struct tm tm; time_t ut = server.unixtime; localtime_r(&ut, &tm); - server.daylight_active = tm.tm_isdst; + atomic_store_explicit(&server.daylight_active, tm.tm_isdst, memory_order_relaxed); } } diff --git a/src/server.h b/src/server.h index 10173daace..f7a2960e5d 100644 --- a/src/server.h +++ b/src/server.h @@ -2114,7 +2114,7 @@ struct valkeyServer { /* time cache */ time_t unixtime; /* Unix time sampled every cron cycle. */ time_t timezone; /* Cached timezone. As set by tzset(). */ - int daylight_active; /* Currently in daylight saving time. */ + _Atomic int daylight_active; /* Currently in daylight saving time. */ mstime_t mstime; /* 'unixtime' in milliseconds. */ ustime_t ustime; /* 'unixtime' in microseconds. */ mstime_t cmd_time_snapshot; /* Time snapshot of the root execution nesting. */