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

New issues in 1.9.0 #2893

Closed
1 task
AdamGerthel opened this issue Jan 13, 2025 · 18 comments · Fixed by #2894
Closed
1 task

New issues in 1.9.0 #2893

AdamGerthel opened this issue Jan 13, 2025 · 18 comments · Fixed by #2894
Labels
bug Something isn't working released

Comments

@AdamGerthel
Copy link

AdamGerthel commented Jan 13, 2025

Description

As you might remember, I was having issue with version 1.7.2 and upward (see #2831). The patch that was supplied to me (see #2831 (comment)) worked great and the issues disappeared from the logs.

A couple of days ago I decided to upgrade to 1.9.0 because I knew the patch had been merged and officially released. However, it seems that 1.9.0 is just as unstable as previous versions, and I'm now seeing many many crashes in production again. I was not able to reproduce the same issue in 1.9.0, so I currently don't know exactly what's happening:

Image

(note, version 1.3.4 etc. are my app's versions)

No other changes have been made to anything related to Skia or React Native in-between these releases, the only relevant change was to go from the patched 1.7.6 version to 1.9.0.

Here's an example stack trace (not sure how helpful this is):

 *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
pid: 0, tid: 21102 >>> com.actnone.blackdust <<<

backtrace:
  #00  pc 0x000000000063145c  /data/app/~~4XxzpzriUirW-2l-fBlGlA==/com.actnone.blackdust-eJHdxzpStQVc3jdw0nEHBA==/split_config.arm64_v8a.apk!librnskia.so (GrResourceCache::notifyARefCntReachedZero(GrGpuResource*, GrIORef<GrGpuResource>::LastRemovedRef)) (BuildId: 5ee36097cf640d7d22d0ba937b03f66310fb10b7)

On a side note:
Our 28-day average crash rate is now so high because of this issue that it will start to impact our exposure in Google Play. I don't know how to even be able to test for these issues before releasing updates because I'm unable to reproduce it with my test devices. All I know is that it's happening a lot in production. These issues can't help make me think that RN Skia really isn't production ready. Obviously this is OSS, so that's where my expectations are, but perhaps the library should be marked as experimental? Or perhaps it just needs a bigger/better suite of automated tests?

React Native Skia Version

1.9.0

React Native Version

0.76.5

Using New Architecture

  • Enabled

Steps to Reproduce

Don't have, at least not at the moment.

Snack, Code Example, Screenshot, or Link to Repository

Don't have, at least not at the moment.

@AdamGerthel AdamGerthel added the bug Something isn't working label Jan 13, 2025
@wcandillon
Copy link
Contributor

I just reviewed all releases from 1.7.7 till 1.9.0, and they only contain a few minor changes, nothing that "should" affect the library stability on Android.
I definitely recommend to stay on 1.7.7 while this is investigated further.
1.10.1 contains a big change (the underlying RN view is different and the scene graph as well) that I would be interested to see if it is also affected by this crash.

I'm really puzzled that there is a difference between 1.7.7 and 1.9.0 since we didn't release any real change in between these version but I will look a bit harder.

In the meantime, any additional information you might provide on the crash (even just a full stack trace maybe), which devices are being affected, Android API Level and so on would be useful.

Also any information on how the library is used (which version of reanimated, which APIs and so on) could be very useful as well.

@wcandillon
Copy link
Contributor

One thing that comes to mind immediately is that 1.7.7 is actually not exactly the same as the patch I gave you (and the more I think about it the more it could make sense, and again it would be the only real difference between the two versions). If I give you a patch like last time, you could report back if the change is stable or not?

@AdamGerthel
Copy link
Author

Sure, I could try reproducing it in the test repository that I made for #2831. I don't know if the crash will occur though, because I've never actually seen the app crash myself.

I've reverted back to 1.7.6 (+patch) in production already, and I don't want to test potential fixes in prod for obvious reasons, but also because we have a Google Play Feature listing coming up in the end of the month and I don't want to risk having these crashes for new buyers.

@wcandillon
Copy link
Contributor

@AdamGerthel this would be the patch: #2894 and is the only genuine difference between the version you had and 1.9.0. Let me know.

To give a bit of context, this workaround was crashing but I was under the impression that after the 1.7.7 fix it was not crashing anymore and I added it back but it looked like I might have been wrong.

@wcandillon
Copy link
Contributor

@AdamGerthel I would appreciate if you could run a small experiment with #2894 but I understand if it's not really possible.

@AdamGerthel
Copy link
Author

@AdamGerthel I would appreciate if you could run a small experiment with #2894 but I understand if it's not really possible.

v1.10.1 + the patch? I think I could do a rollout to 10% or 15% of the users, especially if you're feeling confident that this was the reason.

@wcandillon
Copy link
Contributor

that would work but 1.9.0 + patch would be even more precise (I would recommend this approach)

@AdamGerthel
Copy link
Author

I've started a 20% rollout containing 1.9.0 with the patch applied. Will report back in a day or two.

@wcandillon
Copy link
Contributor

Thank you, really appreciate it

@AdamGerthel
Copy link
Author

Not sure if this helps, but here's a list of the errors reported in the app where I did not use the patch from #2831. Because none of these errors have appeared on builds with the patch, it seems safe to assume that they're all related to those lines of code.

Image

@wcandillon
Copy link
Contributor

what's libworklets? is it reanimated or something else? like margelo/react-native-worklets-core?

@AdamGerthel
Copy link
Author

AdamGerthel commented Jan 13, 2025

what's libworklets? is it reanimated or something else? like margelo/react-native-worklets-core?

Hmm, must be reanimated (~3.16.1). I'm not using margelo's worklet package.

@wcandillon
Copy link
Contributor

ok if these persists in the future (hopefully not), I would love to privately review the code briefly to see if anything jumps out.
1.10.0 does fix another kind of crash but let's move slowly and carefully.

@wcandillon
Copy link
Contributor

@AdamGerthel let me know if you have any reports today.

@AdamGerthel
Copy link
Author

AdamGerthel commented Jan 15, 2025

No crashes so far in production on the following:

1.7.6 + patch from #2831 (comment) (i.e. the rollback I did before releasing patched 1.9.0)
1.9.0 + patch from #2894
1.10.1 + patch from #2894 (I released this in another of our games that use the same Skia implementations)

@wcandillon
Copy link
Contributor

This is great news thank you.

Copy link
Contributor

🎉 This issue has been resolved in version 1.10.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AdamGerthel
Copy link
Author

@wcandillon So it should be safe for me to hop on 1.10.2, correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants