Skip to content

Commit d094fc1

Browse files
TheAngryByrdTheAngryByrd
authored
Fix Disposes not happening on cancellation (#50)
1 parent 485b666 commit d094fc1

File tree

4 files changed

+116
-28
lines changed

4 files changed

+116
-28
lines changed

‎src/IcedTasks/CancellableTaskBuilderBase.fs‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,95 @@ module CancellableTaskBase =
371371
sm.ResumptionDynamicInfo.ResumptionFunc <- cont
372372
false
373373

374+
375+
/// <summary>
376+
/// The entry point for the dynamic implementation of the corresponding operation. Do not use directly, only used when executing quotations that involve tasks or other reflective execution of F# code.
377+
/// </summary>
378+
[<NoEagerConstraintApplication>]
379+
static member inline internal BindDynamicNoCancellation
380+
(
381+
sm:
382+
byref<ResumableStateMachine<CancellableTaskBaseStateMachineData<'TOverall, 'Builder>>>,
383+
awaiter: 'Awaiter,
384+
continuation:
385+
('TResult1 -> CancellableTaskBaseCode<'TOverall, 'TResult2, 'Builder>)
386+
) : bool =
387+
let mutable awaiter = awaiter
388+
389+
let cont =
390+
(CancellableTaskBaseResumptionFunc<'TOverall, 'Builder>(fun sm ->
391+
let result = Awaiter.GetResult awaiter
392+
(continuation result).Invoke(&sm)
393+
))
394+
395+
// shortcut to continue immediately
396+
if Awaiter.IsCompleted awaiter then
397+
cont.Invoke(&sm)
398+
else
399+
sm.ResumptionDynamicInfo.ResumptionData <-
400+
(awaiter :> ICriticalNotifyCompletion)
401+
402+
sm.ResumptionDynamicInfo.ResumptionFunc <- cont
403+
false
404+
405+
406+
/// <summary>Creates A CancellableTask that runs computation, and when
407+
/// computation generates a result T, runs binder res.</summary>
408+
///
409+
/// <remarks>A cancellation check is performed when the computation is executed.
410+
///
411+
/// The existence of this method permits the use of let! in the
412+
/// cancellableTask { ... } computation expression syntax.</remarks>
413+
///
414+
/// <param name="getAwaiter">The computation to provide an unbound result.</param>
415+
/// <param name="continuation">The function to bind the result of computation.</param>
416+
///
417+
/// <returns>A CancellableTask that performs a monadic bind on the result
418+
/// of computation.</returns>
419+
[<NoEagerConstraintApplication>]
420+
member inline internal _.BindNoCancellation
421+
(
422+
awaiter: 'Awaiter,
423+
continuation:
424+
('TResult1 -> CancellableTaskBaseCode<'TOverall, 'TResult2, 'Builder>)
425+
) : CancellableTaskBaseCode<'TOverall, 'TResult2, 'Builder> =
426+
427+
CancellableTaskBaseCode<'TOverall, 'TResult2, 'Builder>(fun sm ->
428+
if __useResumableCode then
429+
//-- RESUMABLE CODE START
430+
// Get an awaiter from the Awaiter
431+
let mutable awaiter = awaiter
432+
433+
let mutable __stack_fin = true
434+
435+
if not (Awaiter.IsCompleted awaiter) then
436+
// This will yield with __stack_yield_fin = false
437+
// This will resume with __stack_yield_fin = true
438+
let __stack_yield_fin = ResumableCode.Yield().Invoke(&sm)
439+
__stack_fin <- __stack_yield_fin
440+
441+
if __stack_fin then
442+
let result = Awaiter.GetResult awaiter
443+
(continuation result).Invoke(&sm)
444+
else
445+
let mutable awaiter = awaiter :> ICriticalNotifyCompletion
446+
447+
MethodBuilder.AwaitUnsafeOnCompleted(
448+
&sm.Data.MethodBuilder,
449+
&awaiter,
450+
&sm
451+
)
452+
453+
false
454+
else
455+
CancellableTaskBuilderBase.BindDynamicNoCancellation(
456+
&sm,
457+
awaiter,
458+
continuation
459+
)
460+
//-- RESUMABLE CODE END
461+
)
462+
374463
/// <summary>Creates A CancellableTask that runs computation, and when
375464
/// computation generates a result T, runs binder res.</summary>
376465
///
@@ -673,7 +762,9 @@ module CancellableTaskBase =
673762
) : CancellableTaskBaseCode<'TOverall, 'T, 'Builder> =
674763
ResumableCode.TryFinallyAsync(
675764
computation,
676-
ResumableCode<_, _>(fun sm -> x.Bind((compensation ()), (x.Zero)).Invoke(&sm))
765+
ResumableCode<_, _>(fun sm ->
766+
x.BindNoCancellation((compensation ()), (x.Zero)).Invoke(&sm)
767+
)
677768
)
678769

679770

‎tests/IcedTasks.Tests/CancellablePoolingValueTaskTests.fs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -510,12 +510,12 @@ module CancellablePoolingValueTaskTests =
510510
testCaseAsync "use IAsyncDisposable cancelled"
511511
<| async {
512512
let data = 42
513-
let mutable wasDisposed = TaskCompletionSource<bool>()
513+
let mutable wasDisposed = false
514514

515515
let doDispose () =
516516
task {
517-
do! Task.Yield()
518-
wasDisposed.SetResult true
517+
do! Task.Delay(15)
518+
wasDisposed <- true
519519
}
520520
|> ValueTask
521521

@@ -537,14 +537,10 @@ module CancellablePoolingValueTaskTests =
537537
timeProvider.ForwardTimeAsync(TimeSpan.FromMilliseconds(100))
538538
|> Async.AwaitTask
539539

540-
let! _ =
540+
do!
541541
Expect.CancellationRequested inProgress
542542
|> Async.AwaitValueTask
543543

544-
let! wasDisposed =
545-
wasDisposed.Task
546-
|> Async.AwaitTask
547-
548544
Expect.isTrue wasDisposed ""
549545
}
550546

‎tests/IcedTasks.Tests/CancellableTaskTests.fs‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ module CancellableTaskTests =
470470
<| async {
471471
let doDispose () =
472472
task {
473-
do! Task.Yield()
473+
do! Task.Delay(15)
474474
failwith "boom"
475475
}
476476
|> ValueTask
@@ -492,16 +492,18 @@ module CancellableTaskTests =
492492
testCaseAsync "use IAsyncDisposable cancelled"
493493
<| async {
494494
let data = 42
495-
let mutable wasDisposed = TaskCompletionSource<bool>()
495+
let mutable wasDisposed = false
496+
497+
let timeProvider = ManualTimeProvider()
496498

497499
let doDispose () =
498500
task {
499-
do! Task.Yield()
500-
wasDisposed.SetResult true
501+
do! Task.Delay(15)
502+
503+
wasDisposed <- true
501504
}
502505
|> ValueTask
503506

504-
let timeProvider = ManualTimeProvider()
505507

506508
let actor data =
507509
cancellableTask {
@@ -519,16 +521,19 @@ module CancellableTaskTests =
519521
timeProvider.ForwardTimeAsync(TimeSpan.FromMilliseconds(100))
520522
|> Async.AwaitTask
521523

524+
Expect.isFalse wasDisposed ""
522525

523-
let! _ =
524-
Expect.CancellationRequested inProgress
526+
do!
527+
timeProvider.ForwardTimeAsync(TimeSpan.FromMilliseconds(100))
525528
|> Async.AwaitTask
526529

527-
let! wasDisposed =
528-
wasDisposed.Task
530+
do!
531+
Expect.CancellationRequested inProgress
529532
|> Async.AwaitTask
530533

534+
531535
Expect.isTrue wasDisposed ""
536+
532537
}
533538

534539

@@ -540,7 +545,7 @@ module CancellableTaskTests =
540545
let doDispose () =
541546
task {
542547
Expect.isFalse wasDisposed ""
543-
do! Task.Yield()
548+
do! Task.Delay(15)
544549
wasDisposed <- true
545550
}
546551
|> ValueTask
@@ -565,7 +570,7 @@ module CancellableTaskTests =
565570
let doDispose () =
566571
task {
567572
Expect.isFalse wasDisposed ""
568-
do! Task.Yield()
573+
do! Task.Delay(15)
569574
wasDisposed <- true
570575
}
571576
|> ValueTask

‎tests/IcedTasks.Tests/CancellableValueTaskTests.fs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,12 @@ module CancellableValueTaskTests =
511511
testCaseAsync "use IAsyncDisposable cancelled"
512512
<| async {
513513
let data = 42
514-
let mutable wasDisposed = TaskCompletionSource<bool>()
514+
let mutable wasDisposed = false
515515

516516
let doDispose () =
517517
task {
518-
do! Task.Yield()
519-
wasDisposed.SetResult true
518+
do! Task.Delay(15)
519+
wasDisposed <- true
520520
}
521521
|> ValueTask
522522

@@ -538,14 +538,10 @@ module CancellableValueTaskTests =
538538
timeProvider.ForwardTimeAsync(TimeSpan.FromMilliseconds(100))
539539
|> Async.AwaitTask
540540

541-
let! _ =
541+
do!
542542
Expect.CancellationRequested inProgress
543543
|> Async.AwaitValueTask
544544

545-
let! wasDisposed =
546-
wasDisposed.Task
547-
|> Async.AwaitTask
548-
549545
Expect.isTrue wasDisposed ""
550546
}
551547

0 commit comments

Comments
 (0)