-
Notifications
You must be signed in to change notification settings - Fork 476
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
Crashing on Android with only a Canvas #1512
Comments
Hi @gustavomts! Thanks for the report and the example. With the latest version I'm not able to make it crash. The memory seems to be released correctly released when the Modal is dismissed. I'm running the profiler on an Android device. The fix we had in 0.1.184 was primarily targeting a regression we had on iOS that was not happening on Android. Could you try to profile the behaviour you're seeing and / or debug to see where or why it is crashing? |
Hi @chrfalch, sure. I'll try to get back to you before the end of the week. |
@chrfalch I have some logs here
In this video, you can see the app increasing the memory usage until it crashes. It takes a while so I had to cut it short, but you can still notice how the memory usage grows. |
public afterMount(): void {
setTimeout(this.toggle, 1000);
}
@bind
public toggle(): void {
this.setState({visible: !Boolean(this.state.visible)}, () => {
setTimeout(this.toggle, 1);
});
}
public render(): any {
if (this.state.visible) {
return (
<Canvas key={'canvas'} style={{ width: '100%', height: 300, display: 'flex'}}>
<Circle cx={128} cy={128} r={128} color="lightblue"/>
</Canvas>
);
}
} I confirm, this code crashes the application in 30 seconds. checked version 0.1.176, the application crashes there too Update <SkiaView onDraw={this.onDraw}/> |
The example was not possible to run out of the box, but I created this example component: export const Breathe = () => {
const [visible, setVisible] = React.useState(true);
const update = () => {
setTimeout(() => {
setVisible((v) => !v);
update();
}, 1000);
};
useEffect(() => {
update();
}, []);
return visible ? (
<Canvas
key={"canvas"}
style={{ width: "100%", height: 300, display: "flex" }}
>
<Circle cx={128} cy={128} r={128} color="lightblue" />
</Canvas>
) : null;
}; When run in Android it refreshes every second, and the memory stays pretty stable over time (we see a small increase in memory but no crash - how long do you need to run this to see the crash? |
With your interval of 1000 milliseconds, I need to wait 1000 times longer, because in my case the interval is 1 millisecond. 1000 * 30 = 30000 seconds Your example will crash in only 8 hours.. In a real application from production, the frequency of re-creating the canvas reaches ~10+ times per second |
Makes sense, I just took the numbers from your example where you used 1000 ms as the timeout :) :) |
We would definetly look for another way to solve this than by recreating the Canvas so often. Could you describe your use case a bit more? |
When scrolling HorizontalList to the right, the canvas jumps into view. Update I rewrote my project to the imperative style and SkiaView, as a result of which the memory leaks disappeared, and the code began to work faster than through Canvas. |
@hitman249 Could you please share your leak-free implementation? I'm curious what the imperative style looks like |
An example is in the documentation |
@hitman249 I would be curious to see the code that runs faster in |
In my issue discussing the crash present in the workaround, I observed that even an empty canvas is gradually accumulating memory. Screen.Recording.2023-08-02.at.11.40.39.mov |
@cjhines do you have a small reproducible example for this? |
@wcandillon Sure! Here's a repo: https://github.com/cjhines/skia-memory-example By repeatedly opening and closing the second screen with the buttons on the top, you will see memory consumption growing rapidly. In our real app this is even worse due to SVG variety. memory.mov |
Thanks, @cjhines - I'm looking into this - the example is a bit big and we'd like to be able to reduce any number of potential causes for this to fail, would it be possible to reduce it to a single source file without any other dependencies than RN Skia (and Reanimated + navigation (stack/tabs)) if possible? Would help us a lot. |
@chrfalch Hugely appreciated! I've pushed a commit to simplify the project to be contained in |
Thanks, appreciate it! |
🎉 This issue has been resolved in version 1.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
For some reason, the app is crashing on Android after opening a modal with Canvas multiple times. Each time the modal is opened the memory usage goes up but it doesn't go back down when it's closed.
This problem was also happening on iOS but it seems like it was fixed in 0.1.184 beta.
This problem didn't seem to exist in v0.1.176 for Android.
Version
0.1.182 and 0.1.185
Steps to reproduce
Basically, you just need to keep opening and closing the modal until the app crashes.
Snack, code example, screenshot, or link to a repository
https://github.com/ivensgustavo/rnskia-android-crash
The text was updated successfully, but these errors were encountered: