MailboxProcessor.PostAndAsyncReply doesn't handle CancellationToken properly

[stmax82]

MailboxProcessor.PostAndAsyncReply can get stuck forever when the parent gets cancelled.

The following starts two asyncs in parallel and then cancels their parent.

One of the asyncs exits by printing "good async exited", while the other one gets stuck in PostAndAsyncReply forever (it never prints "bad async exited").

open System
open System.Threading

type Message = Add of int * int * AsyncReplyChannel<int>

[<EntryPoint>]
let main argv =
  use cancel = new CancellationTokenSource(3000)

  let goodAsync = async {
    try
      for i in Seq.initInfinite (fun i -> i) do
        if i % 10000000 = 0 then
          printfn "good async working..."
    finally
      printfn "good async exited"
  }

  let badAsync (mbox:MailboxProcessor<Message>) = async {
    try
      printfn "bad async working..."
      let! result = mbox.PostAndAsyncReply (fun reply -> Add (1, 2, reply)) // <- stuck in here forever :(
      printfn "%d" result
    finally
      printfn "bad async exited" // <- we never get here
  }

  let mbox = MailboxProcessor.Start(fun inbox -> async {
    let! Add (a, b, reply) = inbox.Receive()
    do! Async.Sleep 1000000
    reply.Reply (a+b)
  }, cancel.Token)

  [goodAsync; badAsync mbox]
  |> Async.Parallel
  |> Async.Ignore
  |> fun x -> Async.Start(x, cancel.Token)

  Console.ReadLine() |> ignore

  0

Tested with VS 2017 15.5.7 and net461 and netcoreapp2.0.. though the behaviour is the same since at least 2014.

With the MailboxProcessor and PostAndAsyncReply being such basic functionality, I think this should be cancelable. Tomas Petricek once wrote:

In F# asynchronous workflows, the CancellationToken object is passed around automatically under the cover. This means that we don't have to do anything special to support cancellation. When running asynchronous workflow, we can give it cancellation token and everything will work automatically.

[dsyme]

@stmax82 Hmmm... in your example did you mean to add an explicit cancellation of the cancel.Token or are you relying on disposing the token with use?

[dsyme]

OK, added

  cancel.Cancel()
  Console.ReadLine() |> ignore

and this repros

[dsyme]

BTW a semi-workaround is to specify a timeout of Int32.MaxValue. I actually thought we had fixed this though, it might be another case of the same problem

      let! result = mbox.PostAndAsyncReply ((fun reply -> Add (1, 2, reply)), timeout = Int32.MaxValue) 

[matthid]

@dsyme This reminds me to

This code looks so dangerous and error prone I probably shouldn't even try to suggest a fix here (besides rewriting the whole thing - obviously)

• Fix issues around MailboxProcessor #3370 (comment)

Well, this code part is just written in a way too complicated way to ever get it bug-free (to be honest I need to look at this for quite some minutes every time again).

• prevent endless async loop on Receive #3498 (comment)

However, without looking at the code, I'd assume that it has to do something with the async PostAndAsyncReply is returning. I'd assume the mailbox itself does shutdown properly, but the returned async is not in that scenario. This obviously is only an assumption.

[stmax82]

@dsyme no need for cancel.Cancel() nor "use" - in my example the CancellationTokenSource has a timeout of 3 seconds, see:

use cancel = new CancellationTokenSource(**3000**)

.. so it automatically cancels the asyncs after 3 secs while the program is waiting in Console.ReadLine.

You can as well use an explicit cancel.Cancel().. though you should add a short delay between Async.Start and cancel.Cancel() because otherwise it might happen that the asyncs get cancelled before execution enters the try-blocks - in which case they would get cancelled without printing the "good / bad async exited" messages.

Thanks for looking into these problems.

[dsyme]

@stmax82 Fix is here: #4477