Merge lp:~facundo/ubuntuone-client/unlock-waiting-for-queue into lp:ubuntuone-client

[Facundo Batista]

Unlock the wait_for_queue when cancelled. (LP: #729158)

Unlock the wait_for_queue when cancelled.

If the command was waiting for the queue when cancelled, it never released the pathlock, because on cancellation it's removed from the queue, so when the queue become active again it didn't receive the "resume".

Tests included.

[lp:~facundo/ubuntuone-client/unlock-waiting-for-queue]

907. By Facundo Batista on 2011-03-07

Fixes 729158

[John O'Brien (jdobrien)]

Looks good and tests pass. You may want to make sure the ActionQueue.cancel doc string is still accurate.

review: Approve

[lp:~facundo/ubuntuone-client/unlock-waiting-for-queue]

908. By Facundo Batista on 2011-03-07

Fixed docstring

[Facundo Batista (facundo)]

Approved with one review.

review: Approve

=== modified file 'tests/syncdaemon/test_action_queue.py'
--- tests/syncdaemon/test_action_queue.py	2011-03-02 19:21:34 +0000
+++ tests/syncdaemon/test_action_queue.py	2011-03-07 15:47:26 +0000
@@ -2092,6 +2092,14 @@
         self.cmd.cancel()
         self.assertTrue(d.called)
 
+    def test_cancel_releases_queue(self):
+        """Cancel unlocks the wait-for-queue deferred."""
+        self.cmd.finish = lambda: None # don't try to unqueue!
+        d = defer.Deferred()
+        self.cmd.wait_for_queue = d
+        self.cmd.cancel()
+        self.assertTrue(d.called)
+
     def test_cancel_cancelled(self):
         """Don't do anything if command already cancelled."""
         called = []
@@ -5019,6 +5027,23 @@
         self._check_finished_ok()
         self.assertTrue(released)
 
+    def test_cancel_while_waiting_queue(self):
+        """Cancel the command while waiting for queue."""
+        # stop the queue, and fake the pathlock to test releasing
+        self.queue.active = False
+        released = []
+        self.cmd._acquire_pathlock = lambda: defer.succeed(
+                                                lambda: released.append(True))
+
+        # let the command go (will stuck because not runnable), and
+        # cancel in the middle
+        self.cmd.go()
+        self.cmd.cancel()
+
+        # all check
+        self._check_finished_ok()
+        self.assertTrue(released)
+
     def test_marker_error_while_pathclocked(self):
         """The marker errbacks while the command is waiting the pathlock."""
         # monkeypatch methods to flag called and test "while running"

=== modified file 'ubuntuone/syncdaemon/action_queue.py'
--- ubuntuone/syncdaemon/action_queue.py	2011-03-02 19:21:34 +0000
+++ ubuntuone/syncdaemon/action_queue.py	2011-03-07 15:47:26 +0000
@@ -1315,9 +1315,8 @@
     def cancel(self):
         """Cancel the command.
 
-        It also triggers the wait_for_condition deferred (but not the
-        wait_for_queue one, as if the queue is not active, it's pointless
-        to release the pathlock for other commands).
+        Also trigger the wait_for_condition and wait_for_queue deferreds, to
+        unlock the command and finally release the pathlock.
 
         Do nothing if already cancelled (as cancellation can come from other
         thread, it can come at any time, so we need to support double
@@ -1330,6 +1329,9 @@
         if self.wait_for_conditions is not None:
             self.wait_for_conditions.callback(True)
             self.wait_for_conditions = None
+        if self.wait_for_queue is not None:
+            self.wait_for_queue.callback(True)
+            self.wait_for_queue = None
         self.cleanup()
         self.finish()