Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IO monad and stack limit #132

Open
haifengkao opened this issue Jun 10, 2023 · 6 comments
Open

IO monad and stack limit #132

haifengkao opened this issue Jun 10, 2023 · 6 comments

Comments

@haifengkao
Copy link
Collaborator

haifengkao commented Jun 10, 2023

I start to see strange EXEC_BAD_ACCESS with ~500 call stack depth.
The call stack contains a lot of runIO
I am suspecting the composed IO monad causes this error.
Maybe it's better to save the handlers into an array

public struct IO<OutputActionType> {
    private let runIO: (AnyActionHandler<OutputActionType>) -> Void

    public init(_ run: @escaping (AnyActionHandler<OutputActionType>) -> Void) {
        self.runIO = run
    }

    public static func pure() -> IO {
        IO { _ in }
    }

    public func run(_ output: AnyActionHandler<OutputActionType>) {
        runIO(output)
    }

    public func run (_ output: @escaping (DispatchedAction<OutputActionType>) -> Void) {
        runIO(.init(output))
    }
}

extension IO: Monoid {
    public static var identity: IO { .pure() }
}

public func <> <OutputActionType>(lhs: IO<OutputActionType>, rhs: IO<OutputActionType>) -> IO<OutputActionType> {
    .init { handler in
        lhs.run(handler)
        rhs.run(handler)
    }
}
@luizmb
Copy link
Member

luizmb commented Jun 10, 2023

Hi @haifengkao,

Thanks for reporting!

Do you have a sample code that reproduces this crash?
Is this happening on new Xcode beta or also in the 14?

The fact that contains runIO could be also caused by a problem in the middleware side-effect itself, right? Are you using async code in this middleware?

@haifengkao
Copy link
Collaborator Author

截圖 2023-06-11 上午3 13 00 截圖 2023-06-11 上午3 16 25
           var monad: IO<ShopAction> = .identity
            
            let MaxCount = 10000000
            for i in 0 ..< MaxCount {
                let io: IO<ShopAction> = .init( {_ in
                    
                    var randomArray = (1...100).map{_ in arc4random()}
                    print(i, randomArray.count)
                    
                    
                    if i == MaxCount - 1 {
                        let stop = 1
                    }
                    
                    if i == MaxCount / 2 {
                        let stop = 2
                    }
                    
                    if i == 1 {
                        let stop = 3
                    }
                })
                monad =  io <> monad
            }
            
            monad.run(.init(ActionHandlerMock()))

The problem is that I wrote a MiddlewareMananger which will dispatch actions to other middlewares, and collects the returning IO. MiddlewareMananger then composes the IO monads into single one and return it to SwiftRex.
When SwiftRex runs the IO, it is going to send new actions, and middlewares will handle the new actions by executing side effects, which will trigger more new actions.
Therefore the call stack becomes ~500 deep

@luizmb
Copy link
Member

luizmb commented Jun 12, 2023

Hi @haifengkao ,

Thanks for sharing the code. I don't have my mac with me currently, so I'm testing on iPad Playgrounds and therefore the diagnostics are not as good as Xcode, of course. But I believe I managed to reproduce the crash with even less code, this for me was enough to crash.

        var monad: IO<AppAction> = .identity
        
        let MaxCount = 10000000
        for i in 0 ..< MaxCount {
            let io: IO<AppAction> = .init( {_ in
            })
            monad =  io <> monad // <-- Crashes when this line is present
        }

Which gave me:

My App Crashed

Failed to launch swift-playgrounds-dev-run.swift-playgrounds-app.cnzoujrlfdbbisaxhymtpguyerri.501.My-App
The operation couldn't be completed. Transaction failed. Process failed to launch. (process launch failed)

Then I simplified the code to prevent mutation (var), and to check whether that would solve it. It did not, but when the amount of iterations is smaller than 12800, it doesn't crash:

let MaxCount = 12800 // 12800 doesn't crash, 12900 crashes
let monad = (0 ..< MaxCount).map { _ in
  IO<AppAction>.init {_ in
  }
}.reduce(IO<AppAction>.identity, <>)

So I'm assuming here this is something with memory in my iPad. Which is strange because the IO instances are not even being called, it's just 13k instances of a tiny class never used. Maybe I can't have good diagnostics on my iPad and I will try properly on my mac, but I wanted to share this meanwhile.

@luizmb
Copy link
Member

luizmb commented Jun 12, 2023

Yeah, having the 13k instances doesn't crash, because if I remove the "reduce" portion it doesn't crash, so indeed the problem is having 13k nested closures when I use <>. I wonder if they were nonescaping the compiler could optimise the call stack and prevent the crash, but this is IO and I need to allow escaping executions. I will make some experiments with the <> code and see how I can optimise this part for a huge number of nested IO closures, but I'm not 100% sure that this should be done anyway.

Let me know your thoughts.

@haifengkao
Copy link
Collaborator Author

haifengkao commented Jun 15, 2023

@luizmb My issue is different than the sample code. An action is dispatched from a view. The action is received by a bridge middleware, the bridge middleware sends another action. The action triggered more actions from a side effect middleware.

Each action sent will go through the final merged IO monad. In the below images, the stack 37->135, 156->214, 236->296 are the recursive calls produced by the IO monad. The number of recursive calls is proportion to the number of middlewares.

Solutions:

  1. If the IO monads store the closures in an array, it won't have the recursive calls anymore. Combing IO is simply combining the arrays.

  2. Another possible way is to avoid combine .identity IO monad. In my design, each action will be handled by at most 1 middleware. If an action should be handled by 2 middlewares, I will convert the action into 2 different actions by bridge middleware. It implies at most one middleware will return non .identity IO monad. If .identity is ignored totally, I think the number of IO monads can be reduced greatly.

  3. Avoid recursive dispatch. If action1 trigger middleware1 to dispatch action2, ... , action2 trigger middleware2 to dispatch action3, ...., action100000 trigger middleware100000 to dispatch action100001, the stack will overflow eventually. The ReduxPipelineWrapper should run the IO returned by middleware1. After it is finished, run the IO returned by middleware2... etc. The downside of this solution is that I won't be able to see the complete "actionX triggers actionY" flow in the call stack.

  4. Avoiding using BridgeMiddlewares. Most of the middlewares in my app are bridge middlewares. If I avoid using bridge middlewares, the number of IO monads can be greatly reduced.

截圖 2023-06-15 下午10 41 19 截圖 2023-06-15 下午10 41 09 截圖 2023-06-15 下午10 40 58 截圖 2023-06-15 下午10 40 48

@luizmb
Copy link
Member

luizmb commented Jul 7, 2023

Hi @haifengkao ,

I'm currently traveling and couldn't take a look into this, as I don't have access to my Macbook here.
If you find a reasonable solution I will be glad to approve the PR and generate a new version. I may need some help with the Cocoapods release, because it's the only thing I can't do without the Macbook.

Otherwise, I'll have some time only in August to check this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants