Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async.Parallel stack overflow on cancellation of ~2000 uncompleted computations #13165

Closed
bartelink opened this issue May 19, 2022 · 2 comments · Fixed by #13186
Closed

Async.Parallel stack overflow on cancellation of ~2000 uncompleted computations #13165

bartelink opened this issue May 19, 2022 · 2 comments · Fixed by #13186
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.

Comments

@bartelink
Copy link

bartelink commented May 19, 2022

When Async.Parallel is handling cancellation (via the token or a computation throwing) with e.g. 2000 items outstanding, the current implementation as of FSharp.Core 6.0.4 is such that the non-tail-recursive impl can blow the stack.

Repro steps

let [<Fact>] ``Async.Parallel blows stack when cancelling many`` () =
    let gen (i : int) = async {
        if i <> 0 then do! Async.Sleep i
        else return failwith (string i) }
    let count = 1800
    let comps = Seq.init count gen
    let result = Async.Parallel(comps, 16) |> Async.Catch |> Async.RunSynchronously
    test <@ match result with
            | Choice2Of2 e -> int e.Message < count
            | x -> failwithf "unexpected %A" x @>

Known workarounds

batch the work with Seq.ChunkBySize 1500, add an outer Async.Parallel call (distributing the degreeOfParallelism over the constituent calls, or using the same value but internally constraining it with Async.Throttle), Array.concat the results.

Related information

(1800 is the approx threshold with SDK 6.0.202 on MacOS x86 with Rider test runner)

Problem appears to be at

worker trampolineHolder |> unfake

@bartelink bartelink added the Bug label May 19, 2022
@bartelink bartelink changed the title Async.Parallel StackOverflowException on cancellation of ~2000 uncompleted computations Async.Parallel stack overflow on cancellation of ~2000 uncompleted computations May 19, 2022
bartelink added a commit to jet/propulsion that referenced this issue May 19, 2022
@vzarytovskii vzarytovskii added the Area-Library Issues for FSharp.Core not covered elsewhere label May 19, 2022
@dsyme dsyme added the Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. label May 23, 2022
@dsyme
Copy link
Contributor

dsyme commented May 23, 2022

@bartelink I've prepped a fix for this

@bartelink
Copy link
Author

bartelink commented May 23, 2022

Excellent, thanks - nice to see it's [appears to be] a [relatively] simple coding slip rather than an actual oversight.
Certainly glad I didn't venture a fix without taking the time it would take to grok the idioms at play!
Added minor comments you're probably in the middle of invalidating

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants