dotnet / dotnet/fsharp

MailboxProcessor.PostAnd(Async)Reply never returns

Open
#6,285 2 comments 3 reactions 1 assignee Claimed by @dsyme View on GitHub
Area-Async Bug Impact-Medium
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 22h
Merged PRs (30d)
144

Description

Below are 9 unit tests, 2 green / 7 red.

### Situation

The red tests show cases where the MailboxProcessor.PostAnd(Async)Reply calls get stuck for eternity due to different reasons. I think things should never get stuck forever.

This might be related to case #4448, which was fixed a while ago, but there seem to be more cases with similar effects.

### Workaround

Minimally set a cancellation token, e.g. `cancellationToken=CancellationToken.None` on the mailbox processor (which is intended only to govern the asynchronous computation running in the mailbox, and not the request-reply to fetch information). This changes the behaviour of `MailboxProcessor.PostAndAsyncReply` and friends to respect cancellation

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)), cancellationToken=CancellationToken.None)

### Repro

Tested with FSharp.Core 4.6.2.

```fsharp
namespace UnitTestProject1

open System
open System.Threading

open Microsoft.VisualStudio.TestTools.UnitTesting

[]
type AsyncCancellationTests () =
// This test is GREEN because both MailboxProcess.Start and PostAndAsyncReply have a cancellation token
// As expected - it throws an OperationCanceledException
[]
member this.Test1() =
use cancel = new CancellationTokenSource(1000)

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)), cancellationToken=cancel.Token)

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
}, cancellationToken=cancel.Token)
with
| :? OperationCanceledException -> ()

// This test is RED because PostAndAsyncReply doesn't get cancelled even though its containing async gets cancelled
// Expected: it should throw an OperationCanceledException
[]
member this.Test2() =
use cancel = new CancellationTokenSource(1000)

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)))

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
}, cancellationToken=cancel.Token)
with
| :? OperationCanceledException -> ()

// This test is fishy GREEN even though its equal to Test2 (RED), except that the MailboxProcessor now gets a CancellationToken.None
// As expected - it throws an OperationCanceledException
[]
member this.Test2_2() =
use cancel = new CancellationTokenSource(1000)

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)), cancellationToken=CancellationToken.None)

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
}, cancellationToken=cancel.Token)
with
| :? OperationCanceledException -> ()

// This test is RED because PostAndAsyncReply doesn't get cancelled when its MailboxProcessor gets cancelled
// Expected: it should throw an OperationCanceledException
[]
member this.Test3() =
use cancel = new CancellationTokenSource(1000)

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)), cancellationToken=cancel.Token)

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
})
with
| :? OperationCanceledException -> ()

// This test is RED because PostAndAsyncReply gets stuck on a MailboxProcessor after it has been cancelled
// Expected: it should throw an OperationCanceledException
[]
member this.Test4() =
use cancel = new CancellationTokenSource()

let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)), cancellationToken=cancel.Token)

cancel.Cancel()
Thread.Sleep(1000)

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
})
with
| :? OperationCanceledException -> ()

// This test is RED because PostAndAsyncReply gets stuck on a disposed MailboxProcessor.
// Expected: it should throw an ObjectDisposedException
[]
member this.Test5() =
let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)))

(mbox :> IDisposable).Dispose()

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
})
with
| :? ObjectDisposedException -> ()

// This test is RED because PostAndReply gets stuck on a disposed MailboxProcessor.
// Expected: it should throw an ObjectDisposedException
[]
member this.Test6() =
let mbox = MailboxProcessor.Start((fun inbox -> Async.Sleep(1000000)))

(mbox :> IDisposable).Dispose()

try
let reply = mbox.PostAndReply(id)
()
with
| :? ObjectDisposedException -> ()

// This test is RED because PostAndReply gets stuck on a MailboxProcessor that has already exited.
// Expected: it should throw some exception.. maybe ObjectDisposed or InvalidOperation?
[]
member this.Test7() =
let mbox = MailboxProcessor.Start((fun inbox -> async { () }))

Thread.Sleep(1000)

try
let reply = mbox.PostAndReply(id)
()
with
| :? ObjectDisposedException -> ()

// This test is RED because PostAndAsyncReply gets stuck on a MailboxProcessor that has already exited.
// Expected: it should throw some exception.. maybe ObjectDisposed or InvalidOperation?
[]
member this.Test8() =
let mbox = MailboxProcessor.Start((fun inbox -> async { () }))

Thread.Sleep(1000)

try
Async.RunSynchronously(async {
let! reply = mbox.PostAndAsyncReply(id)
()
})
with
| :? ObjectDisposedException -> ()
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.