Replies: 19 comments 23 replies
-
|
Sorry about that!
Can you explain this a bit more? Changing back to |
Beta Was this translation helpful? Give feedback.
-
|
I'm not 100% sure that this code a38e04d#diff-52e8a72d908dc1b0d23451f4b256d0244f27c6361facaa5cf73a026f315c25df is the issue but when the app crash it stop there and there is modification here between the 0.23.0 and the 0.24.0. When I do compare both 0.23.0...0.24.0 I don't see what else could cause that crash. What can I do to help? This is just before the crash
|
Beta Was this translation helpful? Give feedback.
-
After bisecting for some time, I found out that my app was crash on every version for the last two months. But the archived version on TestFlight where working, the difference was the dependencies then. So I did retry to bisect and change TCA to a fixed version(0.23.0) and the app stopped crashing. |
Beta Was this translation helpful? Give feedback.
-
|
Looks like the case of running out of stack space, judging by |
Beta Was this translation helpful? Give feedback.
-
|
Export of the thread and the variable with TCA 0.23.0
|
Beta Was this translation helpful? Give feedback.
-
|
Call me crazy, maybe the change you're referring to @mackoj is adding an additional copy ( |
Beta Was this translation helpful? Give feedback.
-
Thanks, for responding. |
Beta Was this translation helpful? Give feedback.
-
Great catch @andreyz for sure this a38e04d#diff-52e8a72d908dc1b0d23451f4b256d0244f27c6361facaa5cf73a026f315c25df (diff of ViewStore between 0.23 and 0.24) show the copy of the app state. And create the crash 😆 |
Beta Was this translation helpful? Give feedback.
-
|
I'm not sure I see why doing It may be a pain to do but if you want to find the exact TCA commit that introduced this problem for you, you may need to do a manual "bisect" where you just pin to lots of TCA commits until you find the problematic one. FWIW the only recent change I can think of that may have introduced another copy is when we coalesced multiple state updates into a single one: 3ca8249#diff-50345ad35b59a32af74fda59695df9c6b1826302d5b41e5d8fbe47477917cc5dR373 However, that change has been in TCA since 0.20.0 :/ |
Beta Was this translation helpful? Give feedback.
-
I think so.
Yes I did say that but I think in the meantime maybe TCA stack utilisation might have grown up a little and since I am already at 20 KB of stack utilisation with a little more utilisation I might been over it.
I can will do it just to be sure. |
Beta Was this translation helpful? Give feedback.
-
|
Sounds good, very interested to see what you find! If it really is that one line change then we'd be happy to revert. |
Beta Was this translation helpful? Give feedback.
-
|
@mackoj I was a little suspicious of the above change from |
Beta Was this translation helpful? Give feedback.
-
|
I do think that revert the |
Beta Was this translation helpful? Give feedback.
-
|
StackOverflow crashes are the worst to spot. Usually EXC_BAD_ACCESS, code=2 with non-nil addresses at random places where it can't crash. |
Beta Was this translation helpful? Give feedback.
-
I can solve it this way but I do think that AppState should be on the heap(stephencelis was the first to talk about this). It did happen to me on various occasions and be face with this issue and each time it's pretty time-consuming. Find the root cause and the conclusion has often been removed feature or add We cannot rely on the hard limit imposed by the OS since our AppState could go over them so easily. What do you think? @tgrapperon @andreyz FWIW I have a hard time foresee what issue putting AppState on the heap could bring. |
Beta Was this translation helpful? Give feedback.
-
|
It's an ongoing issue with Swift that switches paradigms. The runtime hasn't caught up yet and 512kB is often too small, I agree. I think it should be around 5-10MB, like it is on macOS, especially with the new concurrency model that will limit the number of threads. However, if you're using a Anecdotally, this is not proper to TCA. I had this kind of issue in my vanilla UIKit app where I have a value with many nested value fields. I had to turn the whole thing into a class because I still had the issue when extracting parts to pass to functions. Now that we have property-wrappers in function's arguments, I would try to push the |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the explainer. Since we cannot assume that the is limit will change. Do you think there is a reasonable path to solve this issue ? |
Beta Was this translation helpful? Give feedback.
-
|
To my knowledge, there is no silver bullet. If you reverse the issue, the state grows on the stack when you add a new value field. If this field's size on the stack is small (a number, a boolean, some container that stores on the heap like arrays, dictionaries or strings, a CGPoint, etc.), this is not problematic. In the past, I've experimented with StateBindings (#451). Maybe it can be worthwhile to revisit this and automatically put the substate on the heap as we derive it. I have a private implementation using protocols that is faster than the purely functional approach presented in the PR. If there is traction, I may consider open-sourcing it. |
Beta Was this translation helpful? Give feedback.
-
|
I'm going to convert this to a discussion so that it's easier for us to thread messages because there's a lot to discuss here. |
Beta Was this translation helpful? Give feedback.
-
|
A bit more context informations the AppState is 20 KB func bytesFormater(bytes: Int) -> String {
let bf = ByteCountFormatter()
bf.allowedUnits = .useAll
bf.countStyle = .file
bf.includesUnit = true
bf.isAdaptive = true
return bf.string(fromByteCount: Int64(bytes))
}
print("💾 AppState size: \(bytesFormater(bytes: MemoryLayout<AppState>.size))") |
Beta Was this translation helpful? Give feedback.
-
|
@mackoj Have you seen this discussion? #488 Has some fixes using a boxed type. Edit: ah you linked to it in another post :) |
Beta Was this translation helpful? Give feedback.
-
|
Using
|
Beta Was this translation helpful? Give feedback.
-
|
I did try to reproduce the bug in a sample project without success. I will next try bisect on each commit to confirm the source of the issue. But from what I understand from the sample projet it might be something else at play here 🤷♂️ |
Beta Was this translation helpful? Give feedback.
-
|
While benchmarking I noticed If you can get a failing example that would be awesome. I'd be keen to check it out. |
Beta Was this translation helpful? Give feedback.
-
|
I’ve been seeing this crash too trying to upgrade our app from 0.23 to 0.25. It instantly started crashing on launch. However, we have some threading issues in our app - a legacy queue based effects system was accessing our TCA store concurrently from background threads and of course TCA isn’t thread safe. Fortunately we rarely saw memory related crashes until I attempted this upgrade. I’ve gone down the route of eliminating all cross thread store access so we only access the store from the main queue and the problem seems to have gone away. I don’t know why this change seemed to make it more prone to crashing when accessing across threads though. Are you sure you are only accessing the TCA store from a single thread? |
Beta Was this translation helpful? Give feedback.
-
|
I will look into this too thanks 🙏 |
Beta Was this translation helpful? Give feedback.
-
|
You were right the source of the bug Thanks, |
Beta Was this translation helpful? Give feedback.
-
|
We just started testing our app in iOS 16 and these stack overflow seem to happen all the time. |
Beta Was this translation helpful? Give feedback.
-
|
This is a bad news I will try my app on iOS 16. |
Beta Was this translation helpful? Give feedback.
-
|
As it seems to be an ongoing issue, I've drafted a few diagnostic tools this morning. Disclaimer: I'm not a specialist with low-level things, so please consider them with a pinch of salt.
|
Beta Was this translation helpful? Give feedback.
-
|
@mbrandonw, @stephencelis, if this thing works and is fast enough, maybe we could consider emitting runtime warnings if certain thresholds are reached? Like if we reach 90% of the stack's size for example. If the app crashes because of a stack overflow, there will be at least a trace that could provide some insight and workarounds to the user. |
Beta Was this translation helpful? Give feedback.
-
|
I've drafted #1152 following this idea. If someone having this kind of issue could check my branch at .package(url: "https://github.com/tgrapperon/swift-composable-architecture", branch: "runtime-stack-checks")instead to directly check the effect (you will probably need to reset your package cache). |
Beta Was this translation helpful? Give feedback.
-
Hi folks, late to the party but wanna ask a quick question around this tool. We are investigating a stackoverflow issue recently this issue using this diagnose tool, and I noticed that when printing out the size of the State during state change, even if the state has differed a lot, the state remains the same all the time: Wondering if this is usually the case when there isn't any recursing effects on actions. Thanks for all the help 🙇 |
Beta Was this translation helpful? Give feedback.
-
|
The size in memory of state should remain relatively constant. It's introducing heap allocations like copy-on-write wrapper classes and indirect enums that should bring the size down. |
Beta Was this translation helpful? Give feedback.
-
|
Our team is also facing the same issue and I just stumbled upon this bit of documentation that explains how to increase stack size (credit to this StackOverflow (which name is on point in this context ^^))
It seemed a bit too good to be true but it appears to work according to my first tests using @tgrapperon nice utility above. Stack size apparently went from 1032192 to 16777216 when running on iPhone 11, iOS 15.5. It feels like a quick patch but it will give us some air until we can solve this in a more futureproof manner ;) |
Beta Was this translation helpful? Give feedback.
-
|
Unfortunately this trick was indeed too good to be true and seems to only work when deploying directly from xcode as it doesn't seem to survive the archiving phase :/ |
Beta Was this translation helpful? Give feedback.
-
|
We’ve made a few improvements to the library that should help with stack sizes. We haven’t released them yet, but the changes are on main. Could you point your project to main and see if it helps? Also the changes in the reducer protocol beta should help a lot, but that will require you to convert most of your reducers to the protocol style to get the most benefit. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the update @mbrandonw. We are currently still on a Rx based version of the composable architecture but looking forward to switching to the official version soon ;) For now we are managing using @copyonwrite modifiers on large substates :) |
Beta Was this translation helpful? Give feedback.
-
|
Ohh, sorry I missed that. Hope you're able to work around. 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
don't worry you didn't miss it I didn't mention it actually ^^ For now it's managable this way even if the app will still crash if user stacks a large number of screens as we have recursive states. Switching the "official" TCA will be the next big task for me :) |
Beta Was this translation helpful? Give feedback.
-
Hi,
Describe the bug
Since this modification my app crash on device all the time on startup here.
I had to revert to an old version of TCA(0.23.0) to prevent the bug.
Thread 1: EXC_BAD_ACCESS (code=2, address=0x16f8a7ff8)
Environment
Additional context
I did revert a38e04d#diff-52e8a72d908dc1b0d23451f4b256d0244f27c6361facaa5cf73a026f315c25df to
.send($0)in the 0.25.1 and it's not working 🤣.Beta Was this translation helpful? Give feedback.
All reactions