jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-19 07:20:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

vol: Re-evaluate conditons for cond vars

Browse Source 
Most users of cond vars follow this general pattern when waiting for a
condition:

    while (!condition) {
	CV_WAIT(cv, mutex);
    }

But a few places in src/vol do this:

    if (!condition) {
	CV_WAIT(cv, mutex);
    }

It is important to always re-check for the relevant condition after
waiting for a CV, even if it seems like we only need to wait exactly
once, because pthread_cond_wait() is allowed to wake up its caller
spuriously even the CV hasn't been signalled. On Solaris, this can
actually happen if the calling thread is interrupted by a signal.

In VInitPreAttachVolumes() for DAFS, currently this can cause a
segfault if CV_WAIT returns while 'vq' is empty. We will try to
queue_Remove() the head of the queue itself, resulting in vq.head.next
being set to NULL, which will segfault when we try to pull the next
item off of the queue.

We generally cannot be interrupted by a signal when using opr's
softsig, because signals are only delivered to the softsig thread and
blocked in all other threads. It is technically possible to trigger
this situation on Solaris by sending the (unblockable) SIGCANCEL
signal, though this would be very unusual.

To make sure issues like this cannot happen and to avoid weird corner
cases, adjust all of our CV waiters to wait for a CV using a while()
loop or similar pattern. Spurious wakeups may be impossible with LWP,
but just try to make all code use a similar structure to be safe.

Thanks for mvitale@sinenomine.net for finding and investigating the
relevant issue.

Change-Id: Ib188708b909661c28fa1a709306a02b01d3cd6d3
Reviewed-on: https://gerrit.openafs.org/15327
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
This commit is contained in:
Andrew Deason 2023-02-07 22:48:23 -06:00 committed by Benjamin Kaduk
parent 448e82b452
commit 9bc06a0591
1 changed files with 8 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/vol/volume.c
View File

@ -1025,7 +1025,7 @@ VInitPreAttachVolumes(int nthreads, struct volume_init_queue *vq)
while (nthreads) {
/* dequeue next volume */
opr_mutex_enter(&vq->mutex);
if (queue_IsEmpty(vq)) {
while (queue_IsEmpty(vq)) {
opr_cv_wait(&vq->cv, &vq->mutex);
}
vb = queue_First(vq, volume_init_batch);
@ -1195,8 +1195,10 @@ VShutdown_r(void)
if (VInit < 2) {
Log("VShutdown: aborting attach volumes\n");
vinit_attach_abort = 1;
while (VInit < 2) {
VOL_CV_WAIT(&vol_init_attach_cond);
}
}
for (params.n_parts=0, diskP = DiskPartitionList;
diskP; diskP = diskP->next, params.n_parts++);
@ -1320,12 +1322,14 @@ VShutdown_r(void)
if (VInit < 2) {
Log("VShutdown: aborting attach volumes\n");
vinit_attach_abort = 1;
while (VInit < 2) {
#ifdef AFS_PTHREAD_ENV
VOL_CV_WAIT(&vol_init_attach_cond);
#else
LWP_WaitProcess(VInitAttachVolumes);
#endif /* AFS_PTHREAD_ENV */
}
}
Log("VShutdown: shutting down on-line volumes...\n");
vol_shutting_down = 1;