Stack overflow when .concatenating many Effects

pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.

https://www.pointfree.co/collections/composable-architecture

MIT License

12.23k stars 1.42k forks source link

Stack overflow when .concatenating many Effects #160

Closed pcolton closed 4 years ago

pcolton commented 4 years ago

Describe the bug When queueing up a bunch of Effects via .concatenate, it seems that the nesting of .send() calls inside of Store causes a stack overflow. Maybe the answer is just 'don't do that many', but I wanted to at least bring it up in case there's a non-recursive way in Store to achieve the queueing up of synchronous Effects.

To Reproduce Zip up a project that reproduces the behavior: (this is a Mac console app):

group-tasks.zip

Expected behavior non-crash / stack overflow

Screenshots Example of the thread stack when it crashes....

Environment

Xcode 11.5
OS: macOS 10.15.5

klop commented 4 years ago

Seems to be an issue with how synchronous effects are handled in send. Here's a potential fix (haven't tested beyond verifying that it passes tests): https://github.com/klop/swift-composable-architecture/commit/369c91d61165a1a2dcae57a8a104b265e708811c

mbrandonw commented 4 years ago

Hey @klop, that's a really good fix for this problem! Much better to unroll everything with a while loop than recursively call send.

Would you wanna whip that into a PR for us? If you don't have time no worries, we can also do it.

Thanks!

klop commented 4 years ago

@mbrandonw no problem! It's in #163.

pcolton commented 4 years ago

Awesome guys, and thanks @klop for your help!