summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnand Avati <avati@redhat.com>2013-04-05 02:18:06 -0700
committerAnand Avati <avati@redhat.com>2013-04-09 00:09:50 -0700
commitce111f472796d027796b0cc3a4a6f78689f1172d (patch)
tree71426ba932a622a6d05831aa658804a5d0086522
parent1ca50941d693f48e73723b12a1466a70dd272ea2 (diff)
syncenv: be robust against spurious wake()s
In the current implementation, when the callers of synctasks perform a spurious wake() of a sleeping synctask (i.e, an extra wake() soon after a wake() which already woke up a yielded synctask), there is now a possibility of two sync threacs picking up the same synctask. This can result in a crash. The fix is to change ->slept = 0|1 and membership of synctask in runqueue atomically. Today we dequeue a task from the runqueue in syncenv_task(), but reset ->slept = 0 much later in synctask_switchto() in an unlocked manner -- which is safe, when there are no spurious wake()s. However, this opens a race window where, if a second wake() happens after the dequeue, but before setting ->slept = 0, it results in queueing the same synctask in the runqueue once again, and get picked up by a different synctask. This is has been diagnosed to be the crashes in the regression tests of http://review.gluster.org/4784. However that patch still has a spurious wake() [the trigger for this bug] which is yet to be fixed. Change-Id: I9b4b9dd5115d6e62ba45162ae90dd5e917a4f83d BUG: 948686 Signed-off-by: Anand Avati <avati@redhat.com> Reviewed-on: http://review.gluster.org/4795 Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com>
-rw-r--r--libglusterfs/src/syncop.c12
1 files changed, 6 insertions, 6 deletions
diff --git a/libglusterfs/src/syncop.c b/libglusterfs/src/syncop.c
index 876977e2a01..1a335d147e9 100644
--- a/libglusterfs/src/syncop.c
+++ b/libglusterfs/src/syncop.c
@@ -143,10 +143,10 @@ synctask_wake (struct synctask *task)
if (task->slept && task->woken >= task->waitfor)
__run (task);
+
+ pthread_cond_broadcast (&env->cond);
}
pthread_mutex_unlock (&env->mutex);
-
- pthread_cond_broadcast (&env->cond);
}
void
@@ -349,6 +349,10 @@ syncenv_task (struct syncproc *proc)
list_del_init (&task->all_tasks);
env->runcount--;
+ task->woken = 0;
+ task->slept = 0;
+ task->waitfor = 0;
+
task->proc = proc;
}
unlock:
@@ -368,10 +372,6 @@ synctask_switchto (struct synctask *task)
synctask_set (task);
THIS = task->xl;
- task->woken = 0;
- task->slept = 0;
- task->waitfor = 0;
-
#if defined(__NetBSD__) && defined(_UC_TLSBASE)
/* Preserve pthread private pointer through swapcontex() */
task->ctx.uc_flags &= ~_UC_TLSBASE;