Fix pcl-g46: make psync_status queue fields atomic in status_change_thread (#347)
status_change_thread reads psync_status.filestodownload, filestoupload, localisfull, and remoteisfull under statusmutex, while pstatus_download_recalc, pstatus_upload_recalc, and pstatus_set write them without statusmutex (or under a different mutex). TSan reports the race at pqevent.c:274. Replace all writes of these four fields with __atomic_store_n and capture atomic snapshots in status_change_thread before the condition, using the snapshots for both comparison and the status_old update after struct copy. Also use atomic loads in calc_status() for filestodownload and filestoupload. Closes #335 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
3cc42792b5
commit
07e4189bae
|
|
@ -266,14 +266,18 @@ static void status_change_thread(void *ptr) {
|
|||
}
|
||||
statuschanges = 0;
|
||||
uint32_t cur_status = __atomic_load_n(&psync_status.status, __ATOMIC_RELAXED);
|
||||
uint32_t cur_filestodownload = __atomic_load_n(&psync_status.filestodownload, __ATOMIC_RELAXED);
|
||||
uint32_t cur_filestoupload = __atomic_load_n(&psync_status.filestoupload, __ATOMIC_RELAXED);
|
||||
uint8_t cur_localisfull = __atomic_load_n(&psync_status.localisfull, __ATOMIC_RELAXED);
|
||||
uint8_t cur_remoteisfull = __atomic_load_n(&psync_status.remoteisfull, __ATOMIC_RELAXED);
|
||||
if (((status_old.filestodownload > 0) &&
|
||||
(psync_status.filestodownload == 0)) ||
|
||||
((psync_status.filestodownload > 0) &&
|
||||
(cur_filestodownload == 0)) ||
|
||||
((cur_filestodownload > 0) &&
|
||||
(status_old.filestodownload == 0)) ||
|
||||
((status_old.filestoupload > 0) && (psync_status.filestoupload == 0)) ||
|
||||
((psync_status.filestoupload > 0) && (status_old.filestoupload == 0)) ||
|
||||
((psync_status.localisfull != status_old.localisfull)) ||
|
||||
((psync_status.remoteisfull != status_old.remoteisfull)) ||
|
||||
((status_old.filestoupload > 0) && (cur_filestoupload == 0)) ||
|
||||
((cur_filestoupload > 0) && (status_old.filestoupload == 0)) ||
|
||||
((cur_localisfull != status_old.localisfull)) ||
|
||||
((cur_remoteisfull != status_old.remoteisfull)) ||
|
||||
((cur_status != status_old.status) &&
|
||||
((cur_status == PSTATUS_STOPPED) ||
|
||||
(cur_status == PSTATUS_PAUSED) ||
|
||||
|
|
@ -283,6 +287,10 @@ static void status_change_thread(void *ptr) {
|
|||
(status_old.status == PSTATUS_OFFLINE)))) {
|
||||
status_old = psync_status;
|
||||
status_old.status = cur_status;
|
||||
status_old.filestodownload = cur_filestodownload;
|
||||
status_old.filestoupload = cur_filestoupload;
|
||||
status_old.localisfull = cur_localisfull;
|
||||
status_old.remoteisfull = cur_remoteisfull;
|
||||
}
|
||||
pthread_mutex_unlock(&statusmutex);
|
||||
if (!psync_do_run)
|
||||
|
|
|
|||
|
|
@ -130,12 +130,12 @@ static uint32_t calc_status() {
|
|||
}
|
||||
}
|
||||
|
||||
if ((psync_status.filesdownloading || psync_status.filestodownload) &&
|
||||
(psync_status.filesuploading || psync_status.filestoupload))
|
||||
if ((psync_status.filesdownloading || __atomic_load_n(&psync_status.filestodownload, __ATOMIC_RELAXED)) &&
|
||||
(psync_status.filesuploading || __atomic_load_n(&psync_status.filestoupload, __ATOMIC_RELAXED)))
|
||||
return PSTATUS_DOWNLOADINGANDUPLOADING;
|
||||
else if (psync_status.filesdownloading || psync_status.filestodownload)
|
||||
else if (psync_status.filesdownloading || __atomic_load_n(&psync_status.filestodownload, __ATOMIC_RELAXED))
|
||||
return PSTATUS_DOWNLOADING;
|
||||
else if (psync_status.filesuploading || psync_status.filestoupload)
|
||||
else if (psync_status.filesuploading || __atomic_load_n(&psync_status.filestoupload, __ATOMIC_RELAXED))
|
||||
return PSTATUS_UPLOADING;
|
||||
else
|
||||
return PSTATUS_READY;
|
||||
|
|
@ -174,14 +174,14 @@ void pstatus_download_recalc() {
|
|||
"f WHERE t.type=? AND t.itemid=f.id");
|
||||
psql_bind_uint(res, 1, PSYNC_DOWNLOAD_FILE);
|
||||
if ((row = psql_fetch_int(res))) {
|
||||
psync_status.filestodownload = row[0];
|
||||
__atomic_store_n(&psync_status.filestodownload, row[0], __ATOMIC_RELAXED);
|
||||
psync_status.bytestodownload = row[1];
|
||||
} else {
|
||||
psync_status.filestodownload = 0;
|
||||
__atomic_store_n(&psync_status.filestodownload, 0, __ATOMIC_RELAXED);
|
||||
psync_status.bytestodownload = 0;
|
||||
}
|
||||
psql_free(res);
|
||||
if (!psync_status.filestodownload) {
|
||||
if (!__atomic_load_n(&psync_status.filestodownload, __ATOMIC_RELAXED)) {
|
||||
psync_status.downloadspeed = 0;
|
||||
}
|
||||
__atomic_store_n(&psync_status.status, calc_status(), __ATOMIC_RELAXED);
|
||||
|
|
@ -226,7 +226,7 @@ void pstatus_upload_recalc() {
|
|||
free(filename);
|
||||
}
|
||||
psql_free(res);
|
||||
psync_status.filestoupload = filestou;
|
||||
__atomic_store_n(&psync_status.filestoupload, filestou, __ATOMIC_RELAXED);
|
||||
psync_status.bytestoupload = bytestou;
|
||||
if (!filestou)
|
||||
psync_status.uploadspeed = 0;
|
||||
|
|
@ -253,10 +253,10 @@ void pstatus_set(uint32_t statusid, uint32_t status) {
|
|||
statuses[statusid] = status;
|
||||
if (status_waiters)
|
||||
pthread_cond_broadcast(&statuscond);
|
||||
psync_status.remoteisfull =
|
||||
(statuses[PSTATUS_TYPE_ACCFULL] == PSTATUS_ACCFULL_OVERQUOTA);
|
||||
psync_status.localisfull =
|
||||
(statuses[PSTATUS_TYPE_DISKFULL] == PSTATUS_DISKFULL_FULL);
|
||||
__atomic_store_n(&psync_status.remoteisfull,
|
||||
(statuses[PSTATUS_TYPE_ACCFULL] == PSTATUS_ACCFULL_OVERQUOTA), __ATOMIC_RELAXED);
|
||||
__atomic_store_n(&psync_status.localisfull,
|
||||
(statuses[PSTATUS_TYPE_DISKFULL] == PSTATUS_DISKFULL_FULL), __ATOMIC_RELAXED);
|
||||
pthread_mutex_unlock(&status_internal_mutex);
|
||||
status = calc_status();
|
||||
if (__atomic_load_n(&psync_status.status, __ATOMIC_RELAXED) != status) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue