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

[Crash] RCTImageBlurUtils.m BlurRadius Crash #35706

Closed
OskarEichler opened this issue Dec 23, 2022 · 24 comments
Closed

[Crash] RCTImageBlurUtils.m BlurRadius Crash #35706

OskarEichler opened this issue Dec 23, 2022 · 24 comments
Labels
Component: Image Needs: Triage 🔍 Resolution: Fixed A PR that fixes this issue has been merged.

Comments

@OskarEichler
Copy link
Contributor

OskarEichler commented Dec 23, 2022

Description

We are having an issue with RCTImageBlurUtils.m causing a Kernel crash for some images.

The images we identified are all pretty minimal (black and white, with a lot of black pixels). Not sure if this is related and the blur algorithm simply doesn't have enough information to go by, for most other images everything works normal.

I attached some of the images that are causing the crash below. We tried both a blurRadius of 5, as well as 18. Behavior is the same and it always results in a crash:
Thread 3: EXC_BAD_ACCESS (code=1, address=0x13d54c4c8)

Version

0.70.6

Output of npx react-native info

System:
OS: macOS 13.1
CPU: (10) arm64 Apple M1 Max
Memory: 7.53 GB / 64.00 GB
Shell: 5.8.1 - /bin/zsh
Binaries:
Node: 16.14.0 - /usr/local/bin/node
Yarn: 1.22.17 - /usr/local/bin/yarn
npm: 8.19.2 - ~/trackstats/node_modules/.bin/npm
Watchman: Not Found
Managers:
CocoaPods: 1.11.3 - /Users/Oskar/.rbenv/shims/pod
SDKs:
iOS SDK:
Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1
Android SDK: Not Found
IDEs:
Android Studio: 2021.3 AI-213.7172.25.2113.9123335
Xcode: 14.2/14C18 - /usr/bin/xcodebuild
Languages:
Java: 17.0.3.1 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: ^18.2.0 => 18.2.0
react-native: ^0.70.6 => 0.70.6
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

Steps to reproduce

<ImageBackground
        blurRadius={18}
        source={{uri: 'https://i.scdn.co/image/ab67616d0000b2737663b2f75fe4d8fb2cac8c27'}}
/>

<ImageBackground
        blurRadius={5}
        source={{uri: 'https://i.scdn.co/image/ab67616d0000b273d5a219b270d74a266131df18'}}
/>

Both of those images are crashing the application for us (potentially because they are mostly black?)

Snack, code example, screenshot, or link to a repository

Screenshot 2022-12-23 at 10 48 55

crash-log.txt

@OskarEichler
Copy link
Contributor Author

Following up here as this is pretty serious and reproducible crash that occurs on our production apps.

@shohelahamad
Copy link

Good day to everyone. We are also having the same issue on our production app with version RN 0.70.6. any suggestions ?

@OskarEichler
Copy link
Contributor Author

This seems to be a pretty serious issue causing a full kernel panic. Would be great if someone can chime in!

@JenyaIII
Copy link

Faced with the same error, RN 0.71.1, old Arch, Hermes enabled

@ababol
Copy link

ababol commented Jan 31, 2023

I've the same issue ☝️ (blurRadius + black and white image cause a crash)

RN 0.70.7 / hermes enabled

Edit: Actually on my side it seems to only happen in dev mode.

@OskarEichler
Copy link
Contributor Author

Yes it seems to happen for images that don't have enough color information. Black / White / Grey all cause crashes. This is really bad as quite a lot of users are impacted by this. Would be great if someone from the React Native team can chime in here!

@Shaninnik
Copy link
Contributor

Same issue here, started seeing this recently (probably have not seen this before just because it only happens for certain images). RN 0.70.5 / hermes enabled, old arch, production app

@OskarEichler
Copy link
Contributor Author

Pretty serious issue... would be grateful if someone can look into this! 🙌🏼

@AdriaRios
Copy link

We can also reproduce the issue on production:

RN 0.71.6 / hermes enabled

@OskarEichler
Copy link
Contributor Author

@matrush @sammy-SC @steveccable Sorry for tagging you here, but this is a pretty serious kernel crash, and I saw that the three of you previously fixed major issues with this function. Would really appreciate it you can have a look 🙏🏼

@OskarEichler
Copy link
Contributor Author

Hey everyone, I actually figured out what's causing this crash and created a PR with more context. The fix is very simple, and you can apply it via a patch to your existing apps until the PR is merged and you update React Native.

Here is the PR to see the one-line change: #37508

facebook-github-bot pushed a commit that referenced this issue May 23, 2023
Summary:
## Summary:

This PR fixes a kernel crash caused by trying to blur greyscale images, as described in this issue:
#35706 (comment)

## Context:

The CGImageGetBitsPerPixel(imageRef) == 8 expression checks if each pixel of the image is represented by 8 bits.

In an image, each pixel is typically represented by a certain amount of information to store its color. In a grayscale image, for instance, we often use 8 bits per pixel, which allows for 256 different shades of gray (2^8 = 256).

The function vImageBoxConvolve_ARGB8888 works with ARGB images (which stands for Alpha, Red, Green, Blue). If the image is only black & white, it means it's a grayscale image, and hence, it is not compatible with this function, causing the kernel crash.

To prevent the issue, we should also convert grayscale images to ARGB before processing them.

## Changelog:
[IOS] [FIXED] - Fix RCTImageBlurUtils.m Greyscale Crash

Pull Request resolved: #37508

Test Plan:
```
<ImageBackground
        blurRadius={18}
        source={{uri: 'https://i.scdn.co/image/ab67616d0000b2737663b2f75fe4d8fb2cac8c27'}}
/>

<ImageBackground
        blurRadius={5}
        source={{uri: 'https://i.scdn.co/image/ab67616d0000b273d5a219b270d74a266131df18'}}
/>
``

Reviewed By: NickGerleman

Differential Revision: D46071330

Pulled By: javache

fbshipit-source-id: 8c04cbf88d467596c9c8a9de9a380bc10663a0e5
@OskarEichler
Copy link
Contributor Author

Fix has been merged!

@cortinico cortinico added the Resolution: Fixed A PR that fixes this issue has been merged. label May 23, 2023
@OskarEichler
Copy link
Contributor Author

Unfortunately this fix does not seem to reliably resolve the crashes. I created a second PR that addresses the issue:
#37546

@sammy-SC
Copy link
Contributor

sammy-SC commented May 28, 2023

Hello @OskarEichler,

I would like to take a closer look at this but I can't reproduce the crash with your test plan. Is there anything I missed in test plan or is just just rendering those two images?

@OskarEichler
Copy link
Contributor Author

OskarEichler commented May 28, 2023

@sammy-SC Yes correct, applying a blur-radius on those two images causes an immediate crash because the ARGB method can't handle the image. First I was under the impression that it was due to greyscaled images being excluded from the previous step of being converted to ARGB, which is why I opened the following PR: #37508

However, it turned out that they still caused a crash, possibly because of the new UIGraphicsImageRenderer not properly aligning the bitmap data. I opted to reverse back to the older UIGraphicsBeginImageContextWithOptions which works reliably, however might be a bit slower as the API is older: #37546

There could be a path to use the newer UIGraphicsImageRenderer, however as the new API does not provide a lot of customizability it would involve writing more code and dealing with additional complexity, particularly when dealing with memory management in order to manually ensure that the buffer is properly formatted and aligned.

Are you able to reproduce the crash on your side when blurring the images?

@sammy-SC
Copy link
Contributor

@OskarEichler I actually can't reproduce it. I have tried simulator and device both debug and production builds.

@OskarEichler
Copy link
Contributor Author

@sammy-SC Maybe check if you can reproduce it based on what someone else experienced here:
#35364

https://snack.expo.dev/@saurabhkanswalquizizz/imagebackground-blurradius

@sammy-SC
Copy link
Contributor

@OskarEichler
is the screenshot attached to this issue the same as the attached crashlog?
The screenshot has crash on thread 3. The attached crashlog crashes on thread 0 within destructor of FFFastImageView.

@sammy-SC
Copy link
Contributor

@OskarEichler on the screenshot on the first message on this issue, what is value of tempBufferSize? It is not visible on the screenshot and I think it could give me a good pointer to where the crash is coming from.

My current theory is that it is 0. Calling malloc with 0 is legal but calling vImageBoxConvolve_ARGB8888 with tempBuffer of size 0 leads to the same crash you have in the screenshot. We do not guard against that right now.

@OskarEichler
Copy link
Contributor Author

@sammy-SC Just ran it again - right now it's only happening when building to physical device. Please find all of the info below, and let me know if you need anything else.

Screenshot 2023-05-29 at 14 24 35

Error: Thread 32: EXC_BAD_ACCESS (code=1, address=0x1472a0438)

Debug Info:

dataSource	CFDataRef	0x283a8e760	0x0000000283a8e760
imageRef	CGImageRef	0x1380abb60	0x00000001380abb60
buffer1	vImage_Buffer	
data	void *	0x1471d8000	0x00000001471d8000
height	vImagePixelCount	640
width	vImagePixelCount	640
rowBytes	size_t	1280
bytes	size_t	819200
buffer2	vImage_Buffer	
data	void *	0x14b2c8000	0x000000014b2c8000
height	vImagePixelCount	640
width	vImagePixelCount	640
rowBytes	size_t	1280
tempBuffer	void *	0x14b5fc000	0x000000014b5fc000
boxSize	uint32_t	51dataSource	CFDataRef	0x283a8e760	0x0000000283a8e760
imageRef	CGImageRef	0x1380abb60	0x00000001380abb60
buffer1	vImage_Buffer	
data	void *	0x1471d8000	0x00000001471d8000
height	vImagePixelCount	640
width	vImagePixelCount	640
rowBytes	size_t	1280
bytes	size_t	819200
buffer2	vImage_Buffer	
data	void *	0x14b2c8000	0x000000014b2c8000
height	vImagePixelCount	640
width	vImagePixelCount	640
rowBytes	size_t	1280
tempBuffer	void *	0x14b5fc000	0x000000014b5fc000
boxSize	uint32_t	51

@OskarEichler
Copy link
Contributor Author

@sammy-SC If this is not helpful, please let me know how I can export more helpful logs for you! 🙏🏼

@sammy-SC
Copy link
Contributor

@OskarEichler thank you for providing the extra information.
I see you reproduce the crash inside app Songstats. Do you think you could try to repro it inside of RNTester?
I still can't get a repro and without it, I'm just shooting in the dark.

@antoinerousseau
Copy link
Contributor

I can still reproduce the crash, in the simulator even with this fix applied, with this image and blurRadius: 50

@raid5
Copy link

raid5 commented Oct 8, 2024

Is this issue fixed in future React Native versions since this thread went stale?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Image Needs: Triage 🔍 Resolution: Fixed A PR that fixes this issue has been merged.
Projects
None yet
Development

No branches or pull requests