From 3cc42792b550fdb9fd97c54e6947bca9440f1fac Mon Sep 17 00:00:00 2001 From: Levi Neely <141506390+lneely@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:24:35 +0100 Subject: [PATCH] Fix pcl-bga: make psync_status.status accesses atomic (#346) psync_status.status is written in pstatus_set() (pstatus.c:263) after releasing status_internal_mutex, while status_change() (pclsync_lib.cpp:435) reads it concurrently from the callback thread without any lock. TSan reports the race between T9 (write in pstatus_set) and the status callback thread (read in status_change). Replace all reads and writes of psync_status.status with __atomic_load_n/__atomic_store_n (__ATOMIC_RELAXED) across pstatus.c, pqevent.c, and pclsync_lib.cpp. In status_change_thread, capture the atomic value once into cur_status before the condition to avoid multiple inconsistent loads. In status_change(), capture cur_status at entry and use it throughout, also propagating it to the copied status_ struct. Closes #334 Co-authored-by: Levi Neely Co-authored-by: Claude Sonnet 4.6 --- pclsync/pqevent.c | 15 +++++++++------ pclsync/pstatus.c | 12 ++++++------ pclsync_lib.cpp | 20 +++++++++++--------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/pclsync/pqevent.c b/pclsync/pqevent.c index f00ca8b..7bd48d5 100644 --- a/pclsync/pqevent.c +++ b/pclsync/pqevent.c @@ -265,6 +265,7 @@ static void status_change_thread(void *ptr) { pthread_cond_wait(&statuscond, &statusmutex); } statuschanges = 0; + uint32_t cur_status = __atomic_load_n(&psync_status.status, __ATOMIC_RELAXED); if (((status_old.filestodownload > 0) && (psync_status.filestodownload == 0)) || ((psync_status.filestodownload > 0) && @@ -273,14 +274,16 @@ static void status_change_thread(void *ptr) { ((psync_status.filestoupload > 0) && (status_old.filestoupload == 0)) || ((psync_status.localisfull != status_old.localisfull)) || ((psync_status.remoteisfull != status_old.remoteisfull)) || - ((psync_status.status != status_old.status) && - ((psync_status.status == PSTATUS_STOPPED) || - (psync_status.status == PSTATUS_PAUSED) || - (psync_status.status == PSTATUS_OFFLINE) || + ((cur_status != status_old.status) && + ((cur_status == PSTATUS_STOPPED) || + (cur_status == PSTATUS_PAUSED) || + (cur_status == PSTATUS_OFFLINE) || (status_old.status == PSTATUS_STOPPED) || (status_old.status == PSTATUS_PAUSED) || - (status_old.status == PSTATUS_OFFLINE)))) - status_old = psync_status; + (status_old.status == PSTATUS_OFFLINE)))) { + status_old = psync_status; + status_old.status = cur_status; + } pthread_mutex_unlock(&statusmutex); if (!psync_do_run) break; diff --git a/pclsync/pstatus.c b/pclsync/pstatus.c index e26f9b4..60f1aa3 100644 --- a/pclsync/pstatus.c +++ b/pclsync/pstatus.c @@ -164,7 +164,7 @@ void pstatus_init() { } pstatus_download_recalc(); pstatus_upload_recalc(); - psync_status.status = calc_status(); + __atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED); } void pstatus_download_recalc() { @@ -184,7 +184,7 @@ void pstatus_download_recalc() { if (!psync_status.filestodownload) { psync_status.downloadspeed = 0; } - psync_status.status = calc_status(); + __atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED); } void pstatus_upload_recalc() { @@ -230,7 +230,7 @@ void pstatus_upload_recalc() { psync_status.bytestoupload = bytestou; if (!filestou) psync_status.uploadspeed = 0; - psync_status.status = calc_status(); + __atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED); } void pstatus_download_recalc_async() { @@ -259,8 +259,8 @@ void pstatus_set(uint32_t statusid, uint32_t status) { (statuses[PSTATUS_TYPE_DISKFULL] == PSTATUS_DISKFULL_FULL); pthread_mutex_unlock(&status_internal_mutex); status = calc_status(); - if (psync_status.status != status) { - psync_status.status = status; + if (__atomic_load_n(&psync_status.status, __ATOMIC_RELAXED) != status) { + __atomic_store_n(&psync_status.status, status, __ATOMIC_RELAXED); pstatus_send_status_update(); } } @@ -354,6 +354,6 @@ void pstatus_upload_set_speed(uint32_t speed) { } void pstatus_send_update() { - psync_status.status = calc_status(); + __atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED); pstatus_send_status_update(); } diff --git a/pclsync_lib.cpp b/pclsync_lib.cpp index 736a7c6..b89fa1b 100644 --- a/pclsync_lib.cpp +++ b/pclsync_lib.cpp @@ -431,10 +431,12 @@ static void status_change(pstatus_t *status) { char *err; err = (char *)malloc(1024); + uint32_t cur_status = __atomic_load_n(&status->status, __ATOMIC_RELAXED); std::cout << "Down: " << status->downloadstr << "| Up: " << status->uploadstr - << ", status is " << status2string(status->status) << std::endl; + << ", status is " << status2string(cur_status) << std::endl; *clib::pclsync_lib::get_lib().status_ = *status; - if (status->status == PSTATUS_LOGIN_REQUIRED) { + clib::pclsync_lib::get_lib().status_->status = cur_status; + if (cur_status == PSTATUS_LOGIN_REQUIRED) { if (clib::pclsync_lib::get_lib().get_password().empty()) { clib::pclsync_lib::get_lib().read_password(); } @@ -444,20 +446,20 @@ static void status_change(pstatus_t *status) { (int)clib::pclsync_lib::get_lib().save_pass_); clib::pclsync_lib::get_lib().wipe_password(); std::cout << "logging in" << std::endl; - } else if (status->status == PSTATUS_TFA_REQUIRED) { + } else if (cur_status == PSTATUS_TFA_REQUIRED) { if (clib::pclsync_lib::get_lib().get_tfa_code().empty()) { clib::pclsync_lib::get_lib().read_tfa_code(); } psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(), 1 /* trusted */, 0); clib::pclsync_lib::get_lib().wipe_tfa_code(); - } else if (status->status == PSTATUS_BAD_TFA_CODE) { + } else if (cur_status == PSTATUS_BAD_TFA_CODE) { clib::pclsync_lib::get_lib().wipe_tfa_code(); clib::pclsync_lib::get_lib().read_tfa_code(false /* auto_sms */); psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(), 1 /* trusted */, 0); clib::pclsync_lib::get_lib().wipe_tfa_code(); - } else if (status->status == PSTATUS_BAD_LOGIN_DATA) { + } else if (cur_status == PSTATUS_BAD_LOGIN_DATA) { if (!clib::pclsync_lib::get_lib().newuser_) { clib::pclsync_lib::get_lib().read_password(); psync_set_user_pass(clib::pclsync_lib::get_lib().get_username().c_str(), @@ -478,9 +480,9 @@ static void status_change(pstatus_t *status) { } } } - if (status->status == PSTATUS_READY || status->status == PSTATUS_UPLOADING || - status->status == PSTATUS_DOWNLOADING || - status->status == PSTATUS_DOWNLOADINGANDUPLOADING) { + if (cur_status == PSTATUS_READY || cur_status == PSTATUS_UPLOADING || + cur_status == PSTATUS_DOWNLOADING || + cur_status == PSTATUS_DOWNLOADINGANDUPLOADING) { if (!cryptocheck) { cryptocheck = 1; if (clib::pclsync_lib::get_lib().setup_crypto_) { @@ -491,7 +493,7 @@ static void status_change(pstatus_t *status) { } if (clib::pclsync_lib::get_lib().status_callback_) { clib::pclsync_lib::get_lib().status_callback_( - (int)status->status, status2string(status->status)); + (int)cur_status, status2string(cur_status)); } if (err) {