From 56bb20f35b41401321bed2e7858a9ce1c861eb8e Mon Sep 17 00:00:00 2001 From: Aldwin Vlasblom Date: Fri, 6 Apr 2018 10:45:01 +0200 Subject: [PATCH] Improve correctness of Future.hook Closes #234 --- README.md | 41 ++++++++++++++++------------------------- src/hook.js | 34 +++++++++++++++++++++++++--------- test/5.hook.test.js | 20 +++++++++++--------- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index e61b2a77..44698f9d 100644 --- a/README.md +++ b/README.md @@ -1347,31 +1347,22 @@ withConnection( .fork(console.error, console.log); ``` -In the case that a hooked Future is *cancelled* after the resource was acquired, -`dispose` will be executed and immediately cancelled. This means that rejections -which may happen during this disposal are **silently ignored**. To ensure that -resources are disposed during cancellation, you might synchronously dispose -resources in the `cancel` function of the disposal Future: - - -```js -var closeConnection = conn => Future((rej, res) => { - - //We try to dispose gracefully. - conn.flushGracefully(err => { - if(err === null){ - conn.close(); - res(); - }else{ - rej(err); - } - }); - - //On cancel, we force dispose. - return () => conn.close(); - -}); -``` +When a hooked Future is cancelled while acquiring its resource, nothing else +will happen. However, once the resource has been acquired, the `dispose` +Future is guaranteed to run. + +In the case where the consumption Future settles by resolving, rejecting the +disposal is forked and awaited. If the disposal Future rejects or encounters +an exception, it will replace the state of the consumption Future. Otherwise +rejection reason or resolution value of the consumption Future will determine +the final outcome. + +In the case where an exception is encountered during consumption, the disposal +Future will run, but its results are ignored. + +In the case where the consumption or disposal Future is cancelled from outside, +the disposal Future will start and/or finish its computation, but the results +will be ignored. #### finally diff --git a/src/hook.js b/src/hook.js index 2c7e1a1f..8838f80d 100644 --- a/src/hook.js +++ b/src/hook.js @@ -1,3 +1,5 @@ +/* eslint no-param-reassign:0 */ + import {Core, isFuture} from './core'; import {noop, show, showf, partial1, partial2} from './internal/fn'; import {isFunction} from './internal/is'; @@ -33,14 +35,23 @@ Hook.prototype = Object.create(Core); Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ var _this = this, _acquire = this._acquire, _dispose = this._dispose, _consume = this._consume; - var cancel, cancelAcquire = noop, cancelConsume = noop, resource, value, cont = noop; + var cancel, cancelConsume = noop, resource, value, cont = noop; function Hook$done(){ cont(value); } + function Hook$reject(x){ + rej(x); + } + function Hook$consumptionException(e){ - rec(someError('trying to consume resources for a hooked Future', e, _this.toString())); + var rec_ = rec; + cont = noop; + rej = noop; + rec = noop; + Hook$dispose(); + rec_(someError('trying to consume resources for a hooked Future', e, _this.toString())); } function Hook$disposalException(e){ @@ -48,7 +59,6 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ } function Hook$dispose(){ - cancel = noop; var disposal; try{ disposal = _dispose(resource); @@ -58,13 +68,20 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ if(!isFuture(disposal)){ return Hook$disposalException(invalidDisposal(disposal, _dispose, resource)); } - cancel = disposal._interpret(Hook$disposalException, rej, Hook$done); + disposal._interpret(Hook$disposalException, Hook$reject, Hook$done); + cancel = Hook$cancelDisposal; } - function Hook$cancelConsuption(){ + function Hook$cancelConsumption(){ cancelConsume(); Hook$dispose(); - cancel(); + Hook$cancelDisposal(); + } + + function Hook$cancelDisposal(){ + cont = noop; + rec = noop; + rej = noop; } function Hook$consumptionRejected(x){ @@ -81,7 +98,6 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ function Hook$consume(x){ resource = x; - cancel = Hook$cancelConsuption; var consumption; try{ consumption = _consume(resource); @@ -91,6 +107,7 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ if(!isFuture(consumption)){ return Hook$consumptionException(invalidConsumption(consumption, _consume, resource)); } + cancel = Hook$cancelConsumption; cancelConsume = consumption._interpret( Hook$consumptionException, Hook$consumptionRejected, @@ -98,8 +115,7 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){ ); } - cancelAcquire = _acquire._interpret(rec, rej, Hook$consume); - + var cancelAcquire = _acquire._interpret(rec, Hook$reject, Hook$consume); cancel = cancel || cancelAcquire; return function Hook$fork$cancel(){ cancel() }; diff --git a/test/5.hook.test.js b/test/5.hook.test.js index 563e0239..a2313f89 100644 --- a/test/5.hook.test.js +++ b/test/5.hook.test.js @@ -70,11 +70,11 @@ describe('hook()', function(){ }); it('crashes when the second function throws', function(){ - var m = hook(F.resolved, function(){ throw F.resolved }, function(){ throw U.error }); + var m = hook(F.resolved, function(){ return F.resolved }, function(){ throw U.error }); return U.assertCrashed(m, new Error( 'Error came up while trying to consume resources for a hooked Future:\n' + ' Intentional error for unit testing\n\n' + - ' In: Future.hook(Future.of("resolved"), function (){ throw F.resolved }, function (){ throw U.error })\n' + ' In: Future.hook(Future.of("resolved"), function (){ return F.resolved }, function (){ throw U.error })\n' )); }); @@ -152,25 +152,27 @@ describe('hook()', function(){ setTimeout(cancel, 25); }); - it('cancels dispose appropriately', function(done){ - var dispose = Future(function(){ return function(){ return done() } }); + it('does not cancel disposal', function(done){ + var dispose = Future(function(){ return function(){ return done(U.error) } }); var cancel = hook(F.resolved, function(){ return dispose }, function(){ return of('consume') }) ._interpret(done, U.failRej, U.failRes); setTimeout(cancel, 10); + setTimeout(done, 50); }); - it('cancels delayed dispose appropriately', function(done){ - var dispose = Future(function(){ return function(){ return done() } }); + it('does not cancel delayed dispose', function(done){ + var dispose = Future(function(){ return function(){ return done(U.error) } }); var cancel = hook(F.resolved, function(){ return dispose }, function(){ return F.resolvedSlow }) ._interpret(done, U.failRej, U.failRes); - setTimeout(cancel, 25); + setTimeout(cancel, 50); + setTimeout(done, 100); }); - it('immediately runs and cancels the disposal Future when cancelled after acquire', function(done){ + it('runs the disposal Future when cancelled after acquire', function(done){ var cancel = - hook(F.resolved, function(){ return Future(function(){ return function(){ return done() } }) }, function(){ return F.resolvedSlow }) + hook(F.resolved, function(){ return Future(function(){ done() }) }, function(){ return F.resolvedSlow }) ._interpret(done, U.failRej, U.failRes); setTimeout(cancel, 10); });