Skip to content

Commit

Permalink
Change server.daylight_active to an atomic variable (#876)
Browse files Browse the repository at this point in the history
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 <null>:52173615 (valkey-server:x86_64+0x10003c556)
    #1 _serverLog <null>:52173615 (valkey-server:x86_64+0x10003ca89)
    #2 bioProcessBackgroundJobs <null>:52173615 (valkey-server:x86_64+0x1001402c9)

  Previous write of size 4 at 0x000102875c10 by main thread (mutexes: write M0):
    #0 afterSleep <null>:52173615 (valkey-server:x86_64+0x10004989b)
    #1 aeProcessEvents <null>:52173615 (valkey-server:x86_64+0x100031e52)
    #2 main <null>:52173615 (valkey-server:x86_64+0x100064a3c)
    #3 start <null>:52173615 (dyld:x86_64+0xfffffffffff5c365)
    #4 start <null>: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 <[email protected]>
  • Loading branch information
enjoy-binbin authored Aug 14, 2024
1 parent 6cb86ff commit 370bdb3
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
5 changes: 3 additions & 2 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down

0 comments on commit 370bdb3

Please sign in to comment.