Hello Dan Kenigsberg, Francesco Romani, I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/48197 to review the following change. Change subject: executor: Fix thread-unsafe call ...................................................................... executor: Fix thread-unsafe call Deques support thread-safe, appends and pops from either side of the deque, but deque.clear() is not documented as thread-safe, so we should not assume it is. Now we take the condition before invoking clear, and the comment about thread-safety was fixed to match Python documentation. Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Signed-off-by: Nir Soffer <nsof...@redhat.com> Reviewed-on: https://gerrit.ovirt.org/43790 Continuous-Integration: Jenkins CI Reviewed-by: Francesco Romani <from...@redhat.com> Reviewed-by: Dan Kenigsberg <dan...@redhat.com> Bug-Url: https://bugzilla.redhat.com/1279950 --- M lib/vdsm/executor.py 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/48197/2 diff --git a/lib/vdsm/executor.py b/lib/vdsm/executor.py index fc7880e..f63adec 100644 --- a/lib/vdsm/executor.py +++ b/lib/vdsm/executor.py @@ -220,9 +220,10 @@ def __init__(self, max_tasks): self._max_tasks = max_tasks self._tasks = collections.deque() - # deque is thread safe - we can append and pop from both ends without - # additional locking. We need this condition only for waiting. See: - # https://docs.python.org/2.6/library/queue.html#Queue.Full + # Deque supports thread-safe append and pop from both ends. We need + # this condition for waking up threads waiting on an empty queue and + # protecting other methods which are not documented as thread-safe. + # https://docs.python.org/2/library/collections.html#deque-objects self._cond = threading.Condition(threading.Lock()) def put(self, task): @@ -252,4 +253,5 @@ self._cond.wait() def clear(self): - self._tasks.clear() + with self._cond: + self._tasks.clear() -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches