fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Internal exception surfacing when calling `Post` on disposed `MailBoxProcessor`

Open majocha opened this issue 1 year ago • 4 comments

Message: 
System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'Microsoft.Win32.SafeHandles.SafeWaitHandle'.

  Stack Trace: 
SafeHandle.DangerousAddRef(Boolean& success)
Kernel32.SetEvent(SafeWaitHandle handle)
EventWaitHandle.Set()
Mailbox`1.Post(TMsg msg) line 193
FSharpMailboxProcessor`1.Post(TMsg message) line 419

I can see the above exception occasionally in this test: https://github.com/dotnet/fsharp/blob/e6854f5f8d81499742208bdf809bab50944d1d59/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/MailboxProcessorType.fs#L316

I assume the expected behavior here is no exception?

I'll try to come up with consistent minimal repro, but the problem seems to be here: https://github.com/dotnet/fsharp/blob/69fa88b1f7577e6efc5598854cc447887ed5b2e4/src/FSharp.Core/mailbox.fs#L189-L193 ev is disposed and Set will fail.

The fix seems to be to also assign null to pulse when disposing it here: https://github.com/dotnet/fsharp/blob/69fa88b1f7577e6efc5598854cc447887ed5b2e4/src/FSharp.Core/mailbox.fs#L336-L346

majocha avatar Oct 08 '24 10:10 majocha

This happens with timeouts or cancellation enabled, only then a wait handle is used in Mailbox.

majocha avatar Oct 08 '24 14:10 majocha

Minimal repro:

open System.Threading

let cts = new CancellationTokenSource()
let mb =
    MailboxProcessor.Start( (fun inbox ->
        async {
            let! _ = inbox.Receive()
            let! _ = inbox.Receive()
            return ()
        }),
        cancellationToken = cts.Token
    )

// ensure pulse gets created
Thread.Sleep 100

mb.Post 1
mb.Dispose()
mb.Post 2

majocha avatar Oct 08 '24 15:10 majocha

Related to https://github.com/dotnet/fsharp/pull/13036/files#diff-89da65b208fb5d835cded7993284e2c562b2d6530cc320cc030f5079dfc0372b possibly?

T-Gro avatar Oct 14 '24 16:10 T-Gro

I think getting an ObjectDisposedException is the right scenario if the object was disposed. But it should be better controlled by Fsharp.Core and not thrown from internal usage.

T-Gro avatar Oct 14 '24 16:10 T-Gro

Related to https://github.com/dotnet/fsharp/pull/13036/files#diff-89da65b208fb5d835cded7993284e2c562b2d6530cc320cc030f5079dfc0372b possibly?

I think the exception happens in a code path that was there before that PR.

I think getting an ObjectDisposedException is the right scenario if the object was disposed. But it should be better controlled by Fsharp.Core and not thrown from internal usage.

This may be a breaking change. It seems the current standard behavior most of the time is waiting indefinitely on a disposed wait handle. Only on rare occasions where the wait handle is disposed while it's not awaited this here bug happens.

majocha avatar Oct 24 '24 06:10 majocha