Fix cancellation handling

Ensure to cancel the ongoing promise if queue is cancelled.

renderjs.js

@@ -45,18 +45,12 @@
     try {
       result = callback.apply(context, argument_list);
     } catch (e) {
-      return new RSVP.Queue()
-        .push(function returnPushableError() {
-          return RSVP.reject(e);
-        });
+      return new RSVP.Queue(RSVP.reject(e));
     }
     if (result instanceof RSVP.Queue) {
       return result;
     }
-    return new RSVP.Queue()
-      .push(function returnPushableResult() {
-        return result;
-      });
+    return new RSVP.Queue(result);
   }
 
   function readBlobAsDataURL(blob) {
@@ -111,20 +105,10 @@
         try {
           result = callback(evt);
         } catch (e) {
-          result = RSVP.reject(e);
+          return reject(e);
         }
 
-        callback_promise = result;
-        new RSVP.Queue()
-          .push(function waitForEventCallbackResult() {
-            return result;
-          })
-          .push(undefined, function handleEventCallbackError(error) {
-            if (!(error instanceof RSVP.CancellationError)) {
-              canceller();
-              reject(error);
-            }
-          });
+        callback_promise = new RSVP.Queue(result).push(undefined, reject);
       };
 
       target.addEventListener(type, handle_event_callback, useCapture);
@@ -423,10 +407,7 @@
       if (resolved) {
         throw new ResolvedMonitorError();
       }
-      var queue = new RSVP.Queue()
-        .push(function waitForPromiseToMonitor() {
-          return promise_to_monitor;
-        })
+      var queue = new RSVP.Queue(promise_to_monitor)
         .push(function handlePromiseToMonitorSuccess(fulfillmentValue) {
           // Promise to monitor is fullfilled, remove it from the list
           var len = promise_list.length,
@@ -442,13 +423,6 @@
           }
           promise_list = new_promise_list;
         }, function handlePromiseToMonitorError(rejectedReason) {
-          if (rejectedReason instanceof RSVP.CancellationError) {
-            if (!(promise_to_monitor.isFulfilled &&
-                  promise_to_monitor.isRejected)) {
-              // The queue could be cancelled before the first push is run
-              promise_to_monitor.cancel();
-            }
-          }
           reject(rejectedReason);
           throw rejectedReason;
         });
@@ -603,16 +577,16 @@
   };
 
   function runJob(gadget, name, callback, argument_list) {
-    var job_promise = ensurePushableQueue(callback, argument_list, gadget);
     if (gadget.__job_dict.hasOwnProperty(name)) {
       gadget.__job_dict[name].cancel();
     }
+    var job_promise = ensurePushableQueue(callback, argument_list, gadget);
     gadget.__job_dict[name] = job_promise;
-    gadget.__monitor.monitor(new RSVP.Queue()
-      .push(function waitForJobPromise() {
-        return job_promise;
-      })
+    // gadget.__monitor.monitor(job_promise
+    gadget.__monitor.monitor(new RSVP.Queue(job_promise)
       .push(undefined, function handleJobError(error) {
+        // Do not crash monitor if the job has been cancelled
+        // by a new execution
         if (!(error instanceof RSVP.CancellationError)) {
           throw error;
         }
@@ -1200,10 +1174,7 @@
       result = method(url, options, parent_gadget, old_element);
       // Set the HTML context
       if (typeof result.then === 'function') {
-        return new RSVP.Queue()
-          .push(function () {
-            return result;
-          })
+        return new RSVP.Queue(result)
           .push(function setAsyncGadgetInstanceHTMLContext(gadget_instance) {
             return setGadgetInstanceHTMLContext(gadget_instance, options,
                                                 parent_gadget, url,
@@ -1923,11 +1894,8 @@
       embedded_channel,
       declare_method_list_waiting;
 
-    return new RSVP.Queue()
-      .push(function waitForLoadingGadget() {
     // Wait for the loading gadget to be created
-        return wait_for_gadget_loaded;
-      })
+    return new RSVP.Queue(wait_for_gadget_loaded)
       .push(function handleLoadingGadget(result_list) {
         TmpConstructor = result_list[0];
         root_gadget = result_list[1];

test/mutex_test.js

@@ -171,7 +171,7 @@
     var mutex = new Mutex(),
       counter = 0;
     stop();
-    expect(5);
+    expect(4);
     function assertCounter(value) {
       equal(counter, value);
       counter += 1;
@@ -201,7 +201,9 @@
       })
       .push(undefined, function (error) {
         equal(error.message, 'error in callback1');
-        assertCounter(3);
+        // Callback 2 is called before RSVP.all is rejected
+        // Callback 3 is cancelled by RSVP.all
+        assertCounter(2);
       })
       .always(function () {
         start();
@@ -253,6 +255,86 @@
       });
   });
 
+  test('lockAndRun cancel stop first execution', function () {
+    var mutex = new Mutex(),
+      counter = 0;
+    stop();
+    expect(2);
+    function assertCounter(value) {
+      equal(counter, value);
+      counter += 1;
+    }
+    function callback1() {
+      assertCounter(0);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(-999);
+          ok(false, 'Should not reach that code');
+        });
+    }
+    return new RSVP.Queue()
+      .push(function () {
+        var promise = mutex.lockAndRun(callback1);
+        promise.cancel('cancel callback1');
+        return RSVP.delay(200);
+      })
+      .push(function () {
+        assertCounter(1);
+      })
+      .always(function () {
+        start();
+      });
+  });
+
+  test('lockAndRun cancel stop second execution', function () {
+    var mutex = new Mutex(),
+      counter = 0;
+    stop();
+    expect(3);
+    function assertCounter(value) {
+      equal(counter, value);
+      counter += 1;
+    }
+    function callback1() {
+      assertCounter(0);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(1);
+        });
+    }
+    function callback2() {
+      assertCounter(2);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(-999);
+          ok(false, 'Should not reach that code');
+        });
+    }
+    return new RSVP.Queue()
+      .push(function () {
+        return mutex.lockAndRun(callback1);
+      })
+      .push(function () {
+        var promise = mutex.lockAndRun(callback2);
+        promise.cancel('cancel callback2');
+        return RSVP.delay(200);
+      })
+      .push(function () {
+        assertCounter(2);
+      })
+      .always(function () {
+        start();
+      });
+  });
 
   test('lockAndRun cancel does not cancel previous execution', function () {
     var mutex = new Mutex(),

test/renderjs_test.js

@@ -2229,6 +2229,151 @@
       });
   });
 
+  test('mutex prevent concurrent execution', function () {
+    // Subclass RenderJSGadget to not pollute its namespace
+    var Klass = function () {
+      RenderJSGadget.call(this);
+    }, gadget,
+      counter = 0;
+    Klass.prototype = new RenderJSGadget();
+    Klass.prototype.constructor = Klass;
+    Klass.declareMethod = RenderJSGadget.declareMethod;
+
+    gadget = new Klass();
+
+    function assertCounter(value) {
+      equal(counter, value);
+      counter += 1;
+    }
+
+    Klass.declareMethod('testFoo', function (expected_counter) {
+      assertCounter(expected_counter);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(expected_counter + 1);
+          return counter;
+        });
+    }, {mutex: 'foo'});
+
+    // method can be called
+    stop();
+    expect(10);
+    return new RSVP.Queue()
+      .push(function () {
+        return RSVP.all([
+          gadget.testFoo(0),
+          gadget.testFoo(2),
+          gadget.testFoo(4)
+        ]);
+      })
+      .push(function (result_list) {
+        equal(result_list[0], 2);
+        equal(result_list[1], 4);
+        equal(result_list[2], 6);
+        assertCounter(6);
+      })
+      .always(function () {
+        start();
+      });
+  });
+
+  test('mutex first cancellation stop execution', function () {
+    // Subclass RenderJSGadget to not pollute its namespace
+    var Klass = function () {
+      RenderJSGadget.call(this);
+    }, gadget,
+      counter = 0;
+    Klass.prototype = new RenderJSGadget();
+    Klass.prototype.constructor = Klass;
+    Klass.declareMethod = RenderJSGadget.declareMethod;
+
+    gadget = new Klass();
+
+    function assertCounter(value) {
+      equal(counter, value);
+      counter += 1;
+    }
+
+    Klass.declareMethod('testFoo', function (expected_counter) {
+      assertCounter(expected_counter);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(expected_counter + 1);
+          ok(false, 'Should not reach that code');
+        });
+    }, {mutex: 'foo'});
+
+    // method can be called
+    stop();
+    expect(2);
+
+    return new RSVP.Queue()
+      .push(function () {
+        // Immediately cancel the first call
+        gadget.testFoo(0).cancel();
+        return RSVP.delay(200);
+      })
+      .push(function () {
+        assertCounter(1);
+      })
+      .always(function () {
+        start();
+      });
+  });
+
+  test('not mutex first cancellation stop execution', function () {
+    // Subclass RenderJSGadget to not pollute its namespace
+    var Klass = function () {
+      RenderJSGadget.call(this);
+    }, gadget,
+      counter = 0;
+    Klass.prototype = new RenderJSGadget();
+    Klass.prototype.constructor = Klass;
+    Klass.declareMethod = RenderJSGadget.declareMethod;
+
+    gadget = new Klass();
+
+    function assertCounter(value) {
+      equal(counter, value);
+      counter += 1;
+    }
+
+    Klass.declareMethod('testFoo', function (expected_counter) {
+      assertCounter(expected_counter);
+      return new RSVP.Queue()
+        .push(function () {
+          return RSVP.delay(50);
+        })
+        .push(function () {
+          assertCounter(expected_counter + 1);
+          ok(false, 'Should not reach that code');
+        });
+    });
+
+    // method can be called
+    stop();
+    expect(2);
+
+    return new RSVP.Queue()
+      .push(function () {
+        // Immediately cancel the first call
+        gadget.testFoo(0).cancel();
+        return RSVP.delay(200);
+      })
+      .push(function () {
+        assertCounter(1);
+      })
+      .always(function () {
+        start();
+      });
+  });
+
   /////////////////////////////////////////////////////////////////
   // RenderJSGadgetKlass.ready
   /////////////////////////////////////////////////////////////////
@@ -2431,20 +2576,12 @@
     service_status.status = undefined;
 
     klass.declareService(function () {
+      return RSVP.Promise(function () {
         service_status.start_count += 1;
-      return new RSVP.Queue()
-        .push(function () {
         service_status.status = "started";
-          return RSVP.defer().promise;
-        })
-        .push(undefined, function (error) {
+      }, function () {
         service_status.stop_count += 1;
-          if (error instanceof RSVP.CancellationError) {
         service_status.status = "stopped";
-          } else {
-            service_status.status = "error";
-          }
-          throw error;
       });
     });
   }
@@ -3011,21 +3148,13 @@
     service_status.stop_count = 0;
     service_status.status = undefined;
 
-    klass.onEvent('bar', function (evt) {
+    klass.onEvent('bar', function () {
+      return new RSVP.Promise(function () {
         service_status.start_count += 1;
-      return new RSVP.Queue()
-        .push(function () {
         service_status.status = "started";
-          return RSVP.defer().promise;
-        })
-        .push(undefined, function (error) {
+      }, function () {
         service_status.stop_count += 1;
-          if (error instanceof RSVP.CancellationError) {
         service_status.status = "stopped";
-          } else {
-            service_status.status = "error";
-          }
-          throw error;
       });
     });
   }
@@ -3283,21 +3412,13 @@
     service_status.status = undefined;
 
     klass.declareJob(name, function (parameter) {
+      return new RSVP.Promise(function () {
         service_status.start_count += 1;
         service_status.parameter = parameter;
-      return new RSVP.Queue()
-        .push(function () {
         service_status.status = "started";
-          return RSVP.defer().promise;
-        })
-        .push(undefined, function (error) {
+      }, function () {
         service_status.stop_count += 1;
-          if (error instanceof RSVP.CancellationError) {
         service_status.status = "stopped";
-          } else {
-            service_status.status = "error";
-          }
-          throw error;
       });
     });
   }