akkadotnet / akka.net

Canonical actor model implementation for .NET with local + distributed actors in C# and F#.
http://getakka.net
Other
4.72k stars 1.04k forks source link

Stack overflow occurs when I `return! loop()` early from the Actor's `let rec loop() = actor {}.` #7377

Open 0xBruceLiao opened 1 day ago

0xBruceLiao commented 1 day ago

Version Information

My paket.lock info:

RESTRICTION: == net8.0
NUGET
  remote: https://api.nuget.org/v3/index.json
    Akka (1.5.30)
      Akka.Analyzers (>= 0.2.5)
      Microsoft.Extensions.ObjectPool (>= 6.0.33)
      Newtonsoft.Json (>= 13.0.1)
      System.Collections.Immutable (>= 6.0)
      System.Configuration.ConfigurationManager (>= 6.0.1)
      System.Reflection.Emit (>= 4.7)
      System.Threading.Channels (>= 6.0)
    Akka.Analyzers (0.2.5)
    Akka.FSharp (1.5.30)
      Akka (>= 1.5.30)
      FSharp.Core (>= 8.0.400)

Describe the bug

First, I switched from MailboxProcessor to Akka,MailboxProcessor does not have this bug, but when I switched the code to Akka, this exception occurred. After testing, I found that this bug is triggered whenever my code has an early return and also a return at the end of the function,Like this:

let rec loop () =
        actor {
            let! msg = mailbox.Receive()
            match msg with
            | Hello -> printfn "Hello"
            | Bye s ->  return! loop ()  // Here

            return! loop () // Here
        }

Exception:

Stack overflow.  
 at Akka.FSharp.Actors+Combine@250-2[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Invoke(System.__Canon)
   at Akka.FSharp.Actors+Combine@250-2[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Invoke(System.__Canon)
   at Akka.FSharp.Actors+FunActor`2[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].OnReceive(System.Object)
   at Akka.Actor.ActorCell.Invoke(Akka.Actor.Envelope)
   at Akka.Dispatch.Mailbox.Run()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()

To Reproduce

Run the following code and then wait:

open Akka.FSharp
open Akka.Actor
open System

type Msg =
    | Hello
    | Bye of string

let stackOverflowActor (mailbox: Actor<_>) =
    let rec loop () =
        actor {
            let! msg = mailbox.Receive()
            match msg with
            | Hello -> printfn "Hello"
            | Bye s ->  return! loop () // If it's () here, there won't be an exception

            return! loop ()
        }
    loop ()

let system = System.create "system" <| Configuration.load()
let actor = spawn system "stackOverflowActor" stackOverflowActor

for i = 0 to 1000000 do
    actor <! Bye "Crash"

system.WhenTerminated.Wait()

If all branches use return! loop() and there is no return! loop() at the end of the function, then there will be no exception. Alternatively, if only the end of the function uses return! loop(), there will also be no exception.

But I think that this coding style should not produce an exception。

object commented 1 day ago

I experimented with this code using both official Akka F# and Akkling libraries. Here's what I've found.

  1. actor computational expression assumes use of a recursive function (loop in the example above), and like every recursive function it introduces a risk of stack overflow in case F# compiler is not able to do a tail call optimization. More about it here: https://cyanbyfuchsia.wordpress.com/2014/02/12/recursion-and-tail-recursion-in-f/

  2. Because of p.1 I always review use of return! and try to have only single return after mailbox.Receive. I would write the code above like this:

                let! msg = mailbox.Receive()
                return!
                  match msg with
                  | Hello -> loop ()
                  | Bye s -> loop ()

This code works without problems whether I use official Akka F# API or Akkling.

  1. Not only the original code causes stack overflow when using Akka F# API, it also becomes grotesquely slow after running a few thousand cycles. In fact I could not wait until stack overflow exception will be thrown and just terminated the test app after about 20000 messages received by the actor.

  2. The same code worked fine and quickly when using Akkling.

  3. I checked the implementation of Return method in both libraries.

Akka F#:

        member __.ReturnFrom(x) = x
        member __.Return x = Return x

Akkling:

    member __.ReturnFrom (effect: Effect<'Message>) = effect
    member __.Return (value: Effect<'Message>) : Effect<'Message> = value

When using return with a bang (!), ReturnFrom method is called, and both implementations of ReturnFrom are the same. Return member is implemented differently but it should not be used in the code above. But there are some major differences between official Akka F# and Akkling API, so it's more than just the implementation of a single method that affects the execution. I know that Bartosz (Akkling writer) made some optimization improvements in actor computation expression while working on Akkling library.

To summarize:

object commented 1 day ago

Actually I've found an old discussion between myself and @Horusiath about early return (https://github.com/Horusiath/Akkling/issues/149) and misbehavior of the code with early return, and we both agreed that early return is not something expected in expression-based languages like F#, so the code should be rewritten properly.

There are some code examples implementing computation expression with early return, like this one (https://gist.github.com/Savelenko/e721e3fb286f15104e5be2425f9cfe66), but I don't think it should prioritized for F# API.

Aaronontheweb commented 1 hour ago

Can I mark this issue as closed then?