Repository: fluture-js/Fluture (js)

Issue:
Future.finally should run even when the computation is cancelled
Since `Future.finally` is a simplified version of `Future.hook` (one where the `cleanup` does not depend on the resource), it would be better if they both behaved the same way under cancellation.

I propose that the `finally`-computation is started and cancelled immediately when the computation is cancelled, similar to how `hook` behaves.

Pull request:
Fix cancellation behavior of Future.finally
Closes #71

Patch:
diff --git a/README.md b/README.md
index d0f34ff..95b5dec 100644
--- a/README.md
+++ b/README.md
@@ -549,6 +549,10 @@ program('Hello')
 //> "Hello"
 ```
 
+As with [`hook`](#hook); when the Future is cancelled before the *finally
+computation* is running, the *finally computation* is executed and immediately
+cancelled.
+
 ### Consuming Futures
 
 #### fork
diff --git a/fluture.js b/fluture.js
index a8ec7d1..85c941b 100644
--- a/fluture.js
+++ b/fluture.js
@@ -1584,9 +1584,9 @@
 
   //----------
 
-  function FutureFinally(left, right){
-    this._left = left;
-    this._right = right;
+  function FutureFinally(computation, cleanup){
+    this._computation = computation;
+    this._cleanup = cleanup;
   }
 
   FutureFinally.prototype = Object.create(Future.prototype);
@@ -1594,16 +1594,21 @@
   FutureFinally.prototype._f = function FutureFinally$fork(rej, res){
     const _this = this;
     let cancel;
-    const r = _this._left._f(function FutureFinally$fork$rej(e){
-      cancel = _this._right._f(rej, function FutureFinally$fork$rej$res(){ rej(e) });
+    const cancelComputation = _this._computation._f(function FutureFinally$fork$rej(e){
+      cancel = _this._cleanup._f(rej, function FutureFinally$fork$rej$res(){ rej(e) });
     }, function FutureFinally$fork$res(x){
-      cancel = _this._right._f(rej, function FutureFinally$fork$res$res(){ res(x) });
+      cancel = _this._cleanup._f(rej, function FutureFinally$fork$res$res(){ res(x) });
     });
-    return cancel || (cancel = r, function FutureFinally$fork$cancel(){ cancel() });
+    if(cancel) return cancel;
+    cancel = function FutureFinally$fork$cancelBoth(){
+      cancelComputation();
+      _this._cleanup._f(noop, noop)();
+    };
+    return function FutureFinally$fork$cancel(){ cancel() };
   };
 
   FutureFinally.prototype.toString = function FutureFinally$toString(){
-    return `${this._left.toString()}.finally(${this._right.toString()})`;
+    return `${this._computation.toString()}.finally(${this._cleanup.toString()})`;
   };
 
   //----------


Test patch:
diff --git a/test/5.future-finally.test.js b/test/5.future-finally.test.js
index e7f8765..567a6a1 100644
--- a/test/5.future-finally.test.js
+++ b/test/5.future-finally.test.js
@@ -49,6 +49,11 @@ const testInstance = fin => {
       setTimeout(done, 25);
     });
 
+    it('immediately runs and cancels the disposal Future when cancelled early', done => {
+      const cancel = fin(F.resolvedSlow, Future(() => () => done())).fork(U.failRej, U.failRes);
+      setTimeout(cancel, 10);
+    });
+
   });
 
   describe('#toString()', () => {