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 <lkn@darkstar.example.net> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
5aae38cbd8
commit
3cc42792b5
|
|
@ -265,6 +265,7 @@ static void status_change_thread(void *ptr) {
|
||||||
pthread_cond_wait(&statuscond, &statusmutex);
|
pthread_cond_wait(&statuscond, &statusmutex);
|
||||||
}
|
}
|
||||||
statuschanges = 0;
|
statuschanges = 0;
|
||||||
|
uint32_t cur_status = __atomic_load_n(&psync_status.status, __ATOMIC_RELAXED);
|
||||||
if (((status_old.filestodownload > 0) &&
|
if (((status_old.filestodownload > 0) &&
|
||||||
(psync_status.filestodownload == 0)) ||
|
(psync_status.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.filestoupload > 0) && (status_old.filestoupload == 0)) ||
|
||||||
((psync_status.localisfull != status_old.localisfull)) ||
|
((psync_status.localisfull != status_old.localisfull)) ||
|
||||||
((psync_status.remoteisfull != status_old.remoteisfull)) ||
|
((psync_status.remoteisfull != status_old.remoteisfull)) ||
|
||||||
((psync_status.status != status_old.status) &&
|
((cur_status != status_old.status) &&
|
||||||
((psync_status.status == PSTATUS_STOPPED) ||
|
((cur_status == PSTATUS_STOPPED) ||
|
||||||
(psync_status.status == PSTATUS_PAUSED) ||
|
(cur_status == PSTATUS_PAUSED) ||
|
||||||
(psync_status.status == PSTATUS_OFFLINE) ||
|
(cur_status == PSTATUS_OFFLINE) ||
|
||||||
(status_old.status == PSTATUS_STOPPED) ||
|
(status_old.status == PSTATUS_STOPPED) ||
|
||||||
(status_old.status == PSTATUS_PAUSED) ||
|
(status_old.status == PSTATUS_PAUSED) ||
|
||||||
(status_old.status == PSTATUS_OFFLINE))))
|
(status_old.status == PSTATUS_OFFLINE)))) {
|
||||||
status_old = psync_status;
|
status_old = psync_status;
|
||||||
|
status_old.status = cur_status;
|
||||||
|
}
|
||||||
pthread_mutex_unlock(&statusmutex);
|
pthread_mutex_unlock(&statusmutex);
|
||||||
if (!psync_do_run)
|
if (!psync_do_run)
|
||||||
break;
|
break;
|
||||||
|
|
|
||||||
|
|
@ -164,7 +164,7 @@ void pstatus_init() {
|
||||||
}
|
}
|
||||||
pstatus_download_recalc();
|
pstatus_download_recalc();
|
||||||
pstatus_upload_recalc();
|
pstatus_upload_recalc();
|
||||||
psync_status.status = calc_status();
|
__atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED);
|
||||||
}
|
}
|
||||||
|
|
||||||
void pstatus_download_recalc() {
|
void pstatus_download_recalc() {
|
||||||
|
|
@ -184,7 +184,7 @@ void pstatus_download_recalc() {
|
||||||
if (!psync_status.filestodownload) {
|
if (!psync_status.filestodownload) {
|
||||||
psync_status.downloadspeed = 0;
|
psync_status.downloadspeed = 0;
|
||||||
}
|
}
|
||||||
psync_status.status = calc_status();
|
__atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED);
|
||||||
}
|
}
|
||||||
|
|
||||||
void pstatus_upload_recalc() {
|
void pstatus_upload_recalc() {
|
||||||
|
|
@ -230,7 +230,7 @@ void pstatus_upload_recalc() {
|
||||||
psync_status.bytestoupload = bytestou;
|
psync_status.bytestoupload = bytestou;
|
||||||
if (!filestou)
|
if (!filestou)
|
||||||
psync_status.uploadspeed = 0;
|
psync_status.uploadspeed = 0;
|
||||||
psync_status.status = calc_status();
|
__atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED);
|
||||||
}
|
}
|
||||||
|
|
||||||
void pstatus_download_recalc_async() {
|
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);
|
(statuses[PSTATUS_TYPE_DISKFULL] == PSTATUS_DISKFULL_FULL);
|
||||||
pthread_mutex_unlock(&status_internal_mutex);
|
pthread_mutex_unlock(&status_internal_mutex);
|
||||||
status = calc_status();
|
status = calc_status();
|
||||||
if (psync_status.status != status) {
|
if (__atomic_load_n(&psync_status.status, __ATOMIC_RELAXED) != status) {
|
||||||
psync_status.status = status;
|
__atomic_store_n(&psync_status.status, status, __ATOMIC_RELAXED);
|
||||||
pstatus_send_status_update();
|
pstatus_send_status_update();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -354,6 +354,6 @@ void pstatus_upload_set_speed(uint32_t speed) {
|
||||||
}
|
}
|
||||||
|
|
||||||
void pstatus_send_update() {
|
void pstatus_send_update() {
|
||||||
psync_status.status = calc_status();
|
__atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED);
|
||||||
pstatus_send_status_update();
|
pstatus_send_status_update();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -431,10 +431,12 @@ static void status_change(pstatus_t *status) {
|
||||||
char *err;
|
char *err;
|
||||||
err = (char *)malloc(1024);
|
err = (char *)malloc(1024);
|
||||||
|
|
||||||
|
uint32_t cur_status = __atomic_load_n(&status->status, __ATOMIC_RELAXED);
|
||||||
std::cout << "Down: " << status->downloadstr << "| Up: " << status->uploadstr
|
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;
|
*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()) {
|
if (clib::pclsync_lib::get_lib().get_password().empty()) {
|
||||||
clib::pclsync_lib::get_lib().read_password();
|
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_);
|
(int)clib::pclsync_lib::get_lib().save_pass_);
|
||||||
clib::pclsync_lib::get_lib().wipe_password();
|
clib::pclsync_lib::get_lib().wipe_password();
|
||||||
std::cout << "logging in" << std::endl;
|
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()) {
|
if (clib::pclsync_lib::get_lib().get_tfa_code().empty()) {
|
||||||
clib::pclsync_lib::get_lib().read_tfa_code();
|
clib::pclsync_lib::get_lib().read_tfa_code();
|
||||||
}
|
}
|
||||||
psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(),
|
psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(),
|
||||||
1 /* trusted */, 0);
|
1 /* trusted */, 0);
|
||||||
clib::pclsync_lib::get_lib().wipe_tfa_code();
|
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().wipe_tfa_code();
|
||||||
clib::pclsync_lib::get_lib().read_tfa_code(false /* auto_sms */);
|
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(),
|
psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(),
|
||||||
1 /* trusted */, 0);
|
1 /* trusted */, 0);
|
||||||
clib::pclsync_lib::get_lib().wipe_tfa_code();
|
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_) {
|
if (!clib::pclsync_lib::get_lib().newuser_) {
|
||||||
clib::pclsync_lib::get_lib().read_password();
|
clib::pclsync_lib::get_lib().read_password();
|
||||||
psync_set_user_pass(clib::pclsync_lib::get_lib().get_username().c_str(),
|
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 ||
|
if (cur_status == PSTATUS_READY || cur_status == PSTATUS_UPLOADING ||
|
||||||
status->status == PSTATUS_DOWNLOADING ||
|
cur_status == PSTATUS_DOWNLOADING ||
|
||||||
status->status == PSTATUS_DOWNLOADINGANDUPLOADING) {
|
cur_status == PSTATUS_DOWNLOADINGANDUPLOADING) {
|
||||||
if (!cryptocheck) {
|
if (!cryptocheck) {
|
||||||
cryptocheck = 1;
|
cryptocheck = 1;
|
||||||
if (clib::pclsync_lib::get_lib().setup_crypto_) {
|
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_) {
|
if (clib::pclsync_lib::get_lib().status_callback_) {
|
||||||
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) {
|
if (err) {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue