From 5aae38cbd83f21ddba6354fc302dd356d1d037d4 Mon Sep 17 00:00:00 2001 From: Levi Neely <141506390+lneely@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:14:33 +0100 Subject: [PATCH] Fix pcl-96f: make psync_current_time accesses atomic (#345) psync_current_time is a global time_t written by timer_thread without holding timer_mutex, while ptimer_register reads it under timer_mutex. TSan reports the race at ptimer.c:152 (write) and ptimer.c:216 (read). Replace all reads with __atomic_load_n and all writes with __atomic_store_n using __ATOMIC_RELAXED. This covers the race sites in ptimer.c and all external readers in plocalscan.c, pnetlibs.c, and psynclib.c that access psync_current_time without any mutex. Closes #333 Co-authored-by: Levi Neely Co-authored-by: Claude Sonnet 4.6 --- pclsync/plocalscan.c | 14 +++++++------- pclsync/pnetlibs.c | 16 ++++++++-------- pclsync/psynclib.c | 6 +++--- pclsync/ptimer.c | 26 +++++++++++++------------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/pclsync/plocalscan.c b/pclsync/plocalscan.c index 7c3d276..7299bd7 100644 --- a/pclsync/plocalscan.c +++ b/pclsync/plocalscan.c @@ -562,7 +562,7 @@ scanner_scan_folder(const char *localpath, psync_folderid_t folderid, psync_list_for_each_element_call(&dblist, sync_folderlist, list, free); if (localsleepperfolder) { psys_sleep_milliseconds(localsleepperfolder); - if (psync_current_time - starttime >= + if (__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - starttime >= PSYNC_LOCALSCAN_SLEEPSEC_PER_SCAN * 3 / 2) localsleepperfolder = 0; } @@ -977,7 +977,7 @@ static void scanner_scan(int first) { if (localsleepperfolder < 1) localsleepperfolder = 1; } - starttime = psync_current_time; + starttime = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); restartsleep = 1000; restart: @@ -1162,7 +1162,7 @@ restart: for (i = 0; i < SCAN_LIST_CNT; i++) psync_list_for_each_element_call(&scan_lists[i], sync_folderlist, list, free); if (movedfolders) { - starttime = psync_current_time; + starttime = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); restartsleep = 1000; goto restart; } @@ -1172,9 +1172,9 @@ static int scanner_wait() { struct timespec tm; int ret; if (localnotify == 0) - tm.tv_sec = psync_current_time + PSYNC_LOCALSCAN_RESCAN_NOTIFY_SUPPORTED; + tm.tv_sec = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) + PSYNC_LOCALSCAN_RESCAN_NOTIFY_SUPPORTED; else - tm.tv_sec = psync_current_time + PSYNC_LOCALSCAN_RESCAN_INTERVAL; + tm.tv_sec = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) + PSYNC_LOCALSCAN_RESCAN_INTERVAL; tm.tv_nsec = 0; pthread_mutex_lock(&scan_mutex); if (!scan_wakes) @@ -1199,13 +1199,13 @@ static void scanner_thread() { lastscan = 0; while (psync_do_run) { pstatus_wait_statuses_arr(requiredstatuses, ARRAY_SIZE(requiredstatuses)); - if (lastscan + 5 >= psync_current_time) { + if (lastscan + 5 >= __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)) { psys_sleep_milliseconds(2000); pthread_mutex_lock(&scan_mutex); scan_wakes = 0; pthread_mutex_unlock(&scan_mutex); } - lastscan = psync_current_time; + lastscan = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); scanner_scan(w); w = scanner_wait(); } diff --git a/pclsync/pnetlibs.c b/pclsync/pnetlibs.c index aeb9786..43cd7a0 100644 --- a/pclsync/pnetlibs.c +++ b/pclsync/pnetlibs.c @@ -643,21 +643,21 @@ void psync_socket_close_download(psock_t *sock) { */ void psync_account_downloaded_bytes(int unsigned bytes) { - if (current_download_sec == psync_current_time) + if (current_download_sec == __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)) download_bytes_this_sec += bytes; else { uint64_t sum; unsigned long i; download_bytes_sec[download_bytes_off].tm = current_download_sec; download_bytes_sec[download_bytes_off].bytes = download_bytes_this_sec; - current_download_sec = psync_current_time; + current_download_sec = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); download_bytes_this_sec = bytes; download_bytes_off = (download_bytes_off + 1) % PSYNC_SPEED_CALC_AVERAGE_SEC; sum = 0; for (i = 0; i < PSYNC_SPEED_CALC_AVERAGE_SEC; i++) if (download_bytes_sec[i].tm >= - psync_current_time - PSYNC_SPEED_CALC_AVERAGE_SEC) + __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - PSYNC_SPEED_CALC_AVERAGE_SEC) sum += download_bytes_sec[i].bytes; download_speed = sum / PSYNC_SPEED_CALC_AVERAGE_SEC; pstatus_download_set_speed(download_speed); @@ -665,7 +665,7 @@ void psync_account_downloaded_bytes(int unsigned bytes) { } static unsigned long get_download_bytes_this_sec() { - if (current_download_sec == psync_current_time) + if (current_download_sec == __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)) return download_bytes_this_sec; else return 0; @@ -739,7 +739,7 @@ int psync_socket_readall_download_thread(psock_t *sock, void *buff, } static void account_uploaded_bytes(int unsigned bytes) { - if (current_upload_sec == psync_current_time) + if (current_upload_sec == __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)) upload_bytes_this_sec += bytes; else { uint64_t sum; @@ -747,12 +747,12 @@ static void account_uploaded_bytes(int unsigned bytes) { upload_bytes_sec[upload_bytes_off].tm = current_upload_sec; upload_bytes_sec[upload_bytes_off].bytes = upload_bytes_this_sec; upload_bytes_off = (upload_bytes_off + 1) % PSYNC_SPEED_CALC_AVERAGE_SEC; - current_upload_sec = psync_current_time; + current_upload_sec = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); upload_bytes_this_sec = bytes; sum = 0; for (i = 0; i < PSYNC_SPEED_CALC_AVERAGE_SEC; i++) if (upload_bytes_sec[i].tm >= - psync_current_time - PSYNC_SPEED_CALC_AVERAGE_SEC) + __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - PSYNC_SPEED_CALC_AVERAGE_SEC) sum += upload_bytes_sec[i].bytes; upload_speed = sum / PSYNC_SPEED_CALC_AVERAGE_SEC; pstatus_upload_set_speed(upload_speed); @@ -760,7 +760,7 @@ static void account_uploaded_bytes(int unsigned bytes) { } static unsigned long get_upload_bytes_this_sec() { - if (current_upload_sec == psync_current_time) + if (current_upload_sec == __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)) return upload_bytes_this_sec; else return 0; diff --git a/pclsync/psynclib.c b/pclsync/psynclib.c index 5862e17..791befa 100644 --- a/pclsync/psynclib.c +++ b/pclsync/psynclib.c @@ -1997,13 +1997,13 @@ int psync_delete_all_links_file(psync_fileid_t fileid, char **err) { } void psync_cache_links_all() { - if (psync_current_time - links_last_refresh_time >= + if (__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - links_last_refresh_time >= PSYNC_LINKS_REFRESH_INTERVAL) { - links_last_refresh_time = psync_current_time; + links_last_refresh_time = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); cache_links_all(); } else pdbg_logf(D_WARNING, "refreshing link too early %ld", - (unsigned)psync_current_time - links_last_refresh_time); + (unsigned)__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - links_last_refresh_time); } preciever_list_t *psync_list_email_with_access(unsigned long long linkid, diff --git a/pclsync/ptimer.c b/pclsync/ptimer.c index 6067d5f..e94290a 100644 --- a/pclsync/ptimer.c +++ b/pclsync/ptimer.c @@ -73,7 +73,7 @@ static int timer_running = 0; PSYNC_NOINLINE static void timer_sleep_detected(time_t lt) { struct exception_list *e; pdbg_logf(D_NOTICE, "sleep detected, current_time=%lu, last_current_time=%lu", - (unsigned long)psync_current_time, (unsigned long)lt); + (unsigned long)__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED), (unsigned long)lt); pthread_mutex_lock(&timer_ex_mutex); e = sleeplist; while (e) { @@ -130,7 +130,7 @@ PSYNC_NOINLINE static void timer_process_timers(psync_list *timers) { if (!(timer->opts & PTIMER_STOP_AFTER_RUN)) { timer->opts = 0; psync_list_del(l1); - timer->runat = psync_current_time + timer->numsec; + timer->runat = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) + timer->numsec; psync_list_add_tail( &timerlists[timer->level][(timer->runat >> (timer->level * TIMER_ARRAY_SIZE_SHIFT)) % @@ -145,26 +145,26 @@ PSYNC_NOINLINE static void timer_process_timers(psync_list *timers) { static void timer_thread() { psync_list timers; time_t lt; - lt = psync_current_time; + lt = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); while (psync_do_run) { psync_list_init(&timers); psys_sleep_milliseconds(1000); - psync_current_time = psys_time_seconds(); + __atomic_store_n(&psync_current_time, psys_time_seconds(), __ATOMIC_RELAXED); pthread_mutex_lock(&timer_mutex); - timer_prepare_timers(lt, psync_current_time, &timers); + timer_prepare_timers(lt, __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED), &timers); if (nextsecwaiters) pthread_cond_broadcast(&timer_cond); pthread_mutex_unlock(&timer_mutex); if (unlikely(!psync_list_isempty(&timers))) timer_process_timers(&timers); - if (unlikely(psync_current_time - lt >= 25)) + if (unlikely(__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) - lt >= 25)) timer_sleep_detected(lt); - else if (pdbg_unlikely(psync_current_time == lt)) { + else if (pdbg_unlikely(__atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) == lt)) { if (!psync_do_run) break; psys_sleep_milliseconds(1000); } - lt = psync_current_time; + lt = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); } } @@ -173,14 +173,14 @@ void ptimer_init() { for (i = 0; i < TIMER_LEVELS; i++) for (j = 0; j < TIMER_ARRAY_SIZE; j++) psync_list_init(&timerlists[i][j]); - psync_current_time = psys_time_seconds(); + __atomic_store_n(&psync_current_time, psys_time_seconds(), __ATOMIC_RELAXED); prun_thread("timer", timer_thread); timer_running = 1; } time_t ptimer_time() { if (timer_running) - return psync_current_time; + return __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); else return psys_time_seconds(); } @@ -213,7 +213,7 @@ psync_timer_t ptimer_register(psync_timer_callback func, time_t numsec, timer->level = i; timer->opts = 0; pthread_mutex_lock(&timer_mutex); - timer->runat = psync_current_time + numsec; + timer->runat = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED) + numsec; psync_list_add_tail( &timerlists[i][(timer->runat >> (i * TIMER_ARRAY_SIZE_SHIFT)) % TIMER_ARRAY_SIZE], @@ -280,11 +280,11 @@ void ptimer_do_notify_exception() { void ptimer_wait_next_sec() { time_t ctime; pthread_mutex_lock(&timer_mutex); - ctime = psync_current_time; + ctime = __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED); do { nextsecwaiters++; pthread_cond_wait(&timer_cond, &timer_mutex); nextsecwaiters--; - } while (ctime == psync_current_time); + } while (ctime == __atomic_load_n(&psync_current_time, __ATOMIC_RELAXED)); pthread_mutex_unlock(&timer_mutex); }