expo / expo

An open-source framework for making universal native apps with React. Expo runs on Android, iOS, and the web.
https://docs.expo.dev
MIT License
31.26k stars 4.96k forks source link

[expo-video] Component crashing on unmount #29950

Open mihailapuste opened 3 weeks ago

mihailapuste commented 3 weeks ago

Minimal reproducible example

Cannot provide min repo as expo video seems to crash the snack as shown in https://snack.expo.dev/@kimik/765825

What platform(s) does this occur on?

iOS

Did you reproduce this issue in a development build?

Yes

Summary

I'm looking to swap out the existing expo-av video component with the new expo-video one, and everything seems to be fine, however my app crashes whenever I unmount the video player.

The issue seems to appear to be with the release() functionality, which is called on unmount. I initially thought I should try to release before unmounting as well, but that actually triggered the same issue.

This is code from expo

/// useReleasingSharedObject()
    useEffect(() => {
        isFastRefresh.current = false;
        return () => {
            // This will be called on every fast refresh and on unmount, but we only want to release the object on unmount.
            if (!isFastRefresh.current && objectRef.current) {
                objectRef.current?.release(); // <--- commenting this out makes it work fine!
            }
        };
    }, []);
    return object;

The error encoutered is quite non-descriptive, but consistent.

Error: Exception in HostFunction: failed to define internal native state property

So from what I was able to gather, therelease() seems to be causing trouble.

Environment

expo-env-info 1.2.0 environment info: System: OS: macOS 14.2.1 Shell: 5.9 - /bin/zsh Binaries: Node: 21.1.0 - /opt/homebrew/bin/node Yarn: 1.22.19 - /opt/homebrew/bin/yarn npm: 10.2.0 - /opt/homebrew/bin/npm Watchman: 2023.10.23.00 - /opt/homebrew/bin/watchman Managers: CocoaPods: 1.13.0 - /Users/mihailapuste/.rvm/gems/ruby-3.1.2/bin/pod SDKs: iOS SDK: Platforms: DriverKit 23.2, iOS 17.2, macOS 14.2, tvOS 17.2, visionOS 1.0, watchOS 10.2 IDEs: Android Studio: 2023.1 AI-231.9392.1.2311.11330709 Xcode: 15.2/15C500b - /usr/bin/xcodebuild npmPackages: expo: ~51.0.14 => 51.0.14 react: 18.2.0 => 18.2.0 react-dom: ^18.0.0 => 18.2.0 react-native: 0.74.2 => 0.74.2 npmGlobalPackages: eas-cli: 7.1.3 Expo Workflow: bare

Expo Doctor Diagnostics

✔ Check Expo config for common issues ✖ Check package.json for common issues ✖ Check dependencies for packages that should not be installed directly ✔ Check for common project setup issues ✖ Check for issues with metro config ✔ Check npm/ yarn versions ✖ Check Expo config (app.json/ app.config.js) schema ✔ Check native tooling versions ✖ Check that packages match versions required by installed Expo SDK ✔ Check that native modules do not use incompatible support packages ✔ Check for legacy global CLI installed locally ✖ Check that native modules use compatible support package versions for installed Expo SDK

Detailed check results:

Expected package @expo/config-plugins@~8.0.0 Found invalid: @expo/config-plugins@7.9.1 (for more info, run: npm why @expo/config-plugins) Expected package metro@~0.80.8 Found invalid: metro@0.80.5 (for more info, run: npm why metro) Expected package metro-resolver@~0.80.8 Found invalid: metro-resolver@0.80.5 (for more info, run: npm why metro-resolver) Expected package metro-config@~0.80.8 Found invalid: metro-config@0.80.5 (for more info, run: npm why metro-config) Advice: Upgrade dependencies that are using the invalid package versions0.

Error: Problem validating fields in /Users/mihailapuste/fit52/app.json. Learn more: https://docs.expo.dev/workflow/configuration/ • should NOT have additional property 'displayName'.

The package "@types/react-native" should not be installed directly in your project, as types are included with the "react-native" package.

The following scripts in package.json conflict with the contents of node_modules/.bin: lint-staged, tsc.

It looks like that you are using a custom metro.config.js that does not extend @expo/metro-config. This can lead to unexpected and hard to debug issues. Learn more: https://docs.expo.dev/guides/customizing-metro/ Advice: Update your custom metro.config.js to extend @expo/metro-config.

(node:46223) [DEP0040] DeprecationWarning: The punycode module is deprecated. Please use a userland alternative instead. (Use node --trace-deprecation ... to show where the warning was created) The following packages should be updated for best compatibility with the installed expo version: @react-native-community/datetimepicker@7.6.4 - expected version: 8.0.1 @react-native-community/netinfo@11.2.1 - expected version: 11.3.1 @react-native-picker/picker@2.6.1 - expected version: 2.7.5 expo-notifications@0.27.7 - expected version: ~0.28.9 react-native-gesture-handler@2.14.1 - expected version: ~2.16.1 react-native-get-random-values@1.10.0 - expected version: ~1.11.0 react-native-pager-view@6.2.3 - expected version: 6.3.0 react-native-reanimated@3.6.2 - expected version: ~3.10.1 react-native-safe-area-context@4.8.2 - expected version: 4.10.1 react-native-screens@3.29.0 - expected version: 3.31.1 react-native-svg@14.1.0 - expected version: 15.2.0 @babel/core@7.23.9 - expected version: ^7.24.0 @types/react@18.2.48 - expected version: ~18.2.79 typescript@5.0.4 - expected version: ~5.3.3 Your project may not work correctly until you install the expected versions of the packages. Found outdated dependencies Advice: Use 'npx expo install --check' to review and upgrade your dependencies.

expo-bot commented 3 weeks ago

Thank you for filing this issue! This comment acknowledges we believe this may be a bug and there’s enough information to investigate it. However, we can’t promise any sort of timeline for resolution. We prioritize issues based on severity, breadth of impact, and alignment with our roadmap. If you’d like to help move it more quickly, you can continue to investigate it more deeply and/or you can open a pull request that fixes the cause.

mihailapuste commented 1 day ago

@behenate

I upgraded to 1.2.3 and the issue persists. It is also present on android as well as ios. I believe you found a separate issue in https://github.com/expo/expo/pull/30022, and not the one mentioned above.

To reproduce, simply unmounting the video player causes the following error

Error: Exception in HostFunction: failed to define internal native state property

The issue seems to be related to the calling of the release() in the useReleasingSharedObject hook. From testing, removing that release() solves the issue of crashing on unmount. Im sure there are other implications there, but hopefully this somewhat helpful to debug.

Thank you!

behenate commented 9 hours ago

@mihailapuste Sorry to hear that! After merging the PR with the fix it seemed to work every time on my setup. I'll take another look. It's most likely a similar issue in a different spot.