Bind of Async<> within task{} should start on the same thread

[T-Gro]

This addresses #14490 .

I am now of the opinion of the current behavior being a bug, and this change should be treated like a bugfix.

None of the docs for task{} and async{} mention starting work on a new thread, this operation is considered to be done explicitly by user code instead.

The proposed change for 'async{} inside task{}' is also consistent with 'async in async' situation, causing less surprises when moving between task{} and async{}.

When it comes to backwards compatibility issues, GUI/main thread involvement is one thing to have in mind.
In the actual behavior's, the code could start on main thread, do the inner async{} elsewhere, and then return.
The proposed change will keep the same context, which I again see as a bugfix - both for correctness sake, but especially for resource utilization.

[vzarytovskii]

Please, bump FSharp.Core version, since it's a change in behaviour.
@KevinRansom wdyt - is minor version bump enought?

[vzarytovskii]

To be completely fair, I'm still unsure if it was not a desired behaviour (starting on a thread pool).

@dsyme was it a deliberate choice?

[T-Gro]

To be completely fair, I'm still unsure if it was not a desired behaviour (starting on a thread pool).

@dsyme was it a deliberate choice?

For full completeness and why I believe this should be treated like a bug/inconsistency:

async inside Task: Outer task before: 1, inner async: 8, after inner async: 8, outside of the CE: 1
Task inside Task: Outer task before: 1, inner task: 1, after inner task: 1, outside of the CE: 1
async inside async: Outer async before: 1, inner async: 1, after inner async: 1, outside of the CE: 1
Task inside async: Outer async before: 1, inner task: 1, after inner task: 1, outside of the CE: 1

Code:

open System.Threading

let getId() = Thread.CurrentThread.ManagedThreadId

let innerAsync() = async { return getId()  }
let innerTask() = task { return getId()  }

let asyncInTask() =
    let t = task {
        let a = getId()
        let! b = innerAsync()
        let c = getId()
        return $"Outer task before: {a}, inner async: {b}, after inner async: {c}"
    }
    let d = getId()
    $"Async inside Task: {t.Result}, outside of the CE: {d}"

let taskInTask() =
    let t = task {
        let a = getId()
        let! b = innerTask()
        let c = getId()
        return $"Outer task before: {a}, inner task: {b}, after inner task: {c}"
    }
    let d = getId()
    $"Task inside Task: {t.Result}, outside of the CE: {d}"

let asyncInAsync() =
    let t = async {
        let a = getId()
        let! b = innerAsync()
        let c = getId()
        return $"Outer async before: {a}, inner async: {b}, after inner async: {c}"
    }
    let d = getId()
    $"Async inside async: {(t |> Async.StartImmediateAsTask).Result}, outside of the CE: {d}"

let taskInsideAsync() =
    let t = async {
        let a = getId()
        let! b = innerTask() |> Async.AwaitTask
        let c = getId()
        return $"Outer async before: {a}, inner task: {b}, after inner task: {c}"
    }
    let d = getId()
    $"Task inside async: {(t |> Async.StartImmediateAsTask).Result}, outside of the CE: {d}"

printfn "%s" (asyncInTask())
printfn "%s" (taskInTask())
printfn "%s" (asyncInAsync())
printfn "%s" (taskInsideAsync())

[abelbraaksma]

@T-Gro that’s an excellent summary. I agree, this looks like a minor oversight and should be fixed, ideally in a point release (I assume we won’t publish fixes on the 6.0.x branch anymore, right? This’ll be just 7.0.x? As that’d be an even greater reason in my book to not wait until 8.x).

Do note that similar wrong behaviour exists for running an async computation and hopping to a different thread immediately (as opposed to when nested), see original issue for details.

[vzarytovskii]

As discussed offline, since it's an implementation detail change - not really explicitly breaking.
I'm just wondering what's what the thinking behind having current behaviour in the first place. Will discuss with Don in NY.

[abelbraaksma]

To answer my own question, and for posterity, this fix will only live in the F# Core 7.x.x versions published after today, there will not be a new 6.0.x (afaik).

[T-Gro]

To answer my own question, and for posterity, this fix will only live in the F# Core 7.x.x versions published after today, there will not be a new 6.0.x (afaik).

Sorry for the radio silence -> yes, this is the plan.

I do not see this as a 'showstopper' kind of bug that should be backported, even though we technically can if this causes enough pain.

[dsyme]

I agree with the fix :) Nice job