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

[3.7.0] Data type not recognized by value unpacker. at valueUnpacker (WorkletRuntime::WorkletRuntime:1:1477) #5660

Closed
efstathiosntonas opened this issue Feb 12, 2024 · 80 comments · Fixed by #5759
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided

Comments

@efstathiosntonas
Copy link
Contributor

efstathiosntonas commented Feb 12, 2024

Description

Cannot reproduce it locally, for now I've encountered in production builds on Androids 11,12,13,14.

Error:
Data type not recognized by value unpacker. at valueUnpacker (WorkletRuntime::WorkletRuntime:1:1477)

Full stacktrace:

com.facebook.jni.CppException: [Reanimated] Data type not recognized by value unpacker.

Error: [Reanimated] Data type not recognized by value unpacker.
    at valueUnpacker (WorkletRuntime::WorkletRuntime:1:1477)
        at com.swmansion.reanimated.AndroidUIScheduler.triggerUI(AndroidUIScheduler.java:-2)
        at com.swmansion.reanimated.AndroidUIScheduler$1.run(AndroidUIScheduler.java:24)
        at com.swmansion.reanimated.AndroidUIScheduler$2.runGuarded(AndroidUIScheduler.java:43)
        at com.facebook.react.bridge.GuardedRunnable.run(GuardedRunnable.java:29)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:250)
        at android.app.ActivityThread.main(ActivityThread.java:7755)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:958)

Steps to reproduce

  1. Updated to 3.7.0 from 3.6.2
  2. crashes

Snack or a link to a repository

no-repo-cannot-reproduce

Reanimated version

3.7.0

React Native version

0.73.4

Platforms

Android

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Paper (Old Architecture)

Build type

Release app & production bundle

Device

Real device

Device model

LG G8 ThinQ™ (LM-G820), Mi Note 10, Galaxy S22 Ultra (SM-S908U), moto g(8) power

Acknowledgements

Yes

@github-actions github-actions bot added the Missing repro This issue need minimum repro scenario label Feb 12, 2024
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@github-actions github-actions bot added the Platform: Android This issue is specific to Android label Feb 12, 2024
@joe-sam
Copy link

joe-sam commented Feb 12, 2024

I don't know if this is related but while testing a PR #5216 I found that some AP initWithContext/NativeProxy I had to add a non optional parameter which is a valueUnpackerCode of type String. You may have to include this missing argument/type in some function call or provide its default value.

@efstathiosntonas
Copy link
Contributor Author

@joe-sam thanks for the heads up, can you please be more specific? What is AP?

where this code should be placed?

public void initWithContext(ReactApplicationContext reactApplicationContext, String valueUnpackerCode) {
   mReactApplicationContext = reactApplicationContext;
   mNativeProxy = new NativeProxy(reactApplicationContext, valueUnpackerCode);
   mAnimationManager.setAndroidUIScheduler(getNativeProxy().getAndroidUIScheduler());
   compatibility = new ReaCompatibility(reactApplicationContext);
   compatibility.registerFabricEventListener(this);
 }

@joe-sam
Copy link

joe-sam commented Feb 13, 2024

@efstathiosntonas

AP was a typo I meant API interface. valueUnpackerCode is a new argument to initContext/nativeProxy. You possibly may have multiple mismatched library versions that might be trying to invoke the reanimated API from the earlier/later library interface.
I had to pin the resolutions for the peer dependency in my package.json before my incompatibility issues went away.

This code snippet comes from here .

@anders-friis-topdk
Copy link

This issue is causing many crashes in my project after updating to React Native 0.73.4, Reanimated 3.7.0 and old architecture. It crashes often but not consistently so it's hard to come up with a repro but I am looking into it.
I also tried downgrading to previous Reanimated version, but it doesn't seem to be compatible with RN 0.73.4 so we are stuck with these crashes at the moment.
I checked our yarn.lock file and it doesn't seem there are multiple reanimated versions in play.

Can you elaborate on the potential fix your mention with initWithContext, @efstathiosntonas or @joe-sam ?

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Feb 14, 2024

@anders-friis-topdk use version 3.6.2, it works with rn 0.73.x.

unfortunately I do not have the time to look into this at this moment, I've downgraded to 3.6.2 for the time being.

edit: I used 3.7.0 in the entire project, I did not have conflicting versions (otherwise the app wouldn't run at all)

@anders-friis-topdk
Copy link

@efstathiosntonas
I just tried downgrading to 3.6.2 and this time I succeeded. Must have been some cache problem last time I tried.
I cannot get the app to crash now so that seems to solve it for now :) Thanks!

@joe-sam
Copy link

joe-sam commented Feb 14, 2024

I am on RN 0.72.10.
I found that multiple package.json peer libraries use different peer dependencies of react-native-reanimated which were completely version confused although this did not show up in yarn.lock
So when resolving I did the following standard methods to try to flush the cache(s?) and pin the version. This worked locally in __DEV__ but don't know whether this works with PROD.
I also pinned the peer dependency resolution of reanimated library by injecting this into package.json


   "resolutions": {
    "react-native-reanimated": "3.7.0"
   }

@efstathiosntonas
Copy link
Contributor Author

@joe-sam did you managed to check if your prod version crash after pinning the version in resolutions?

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Feb 16, 2024

@tjzel @tomekzaw hey guys, care to explain why this might be throwing and if there's a way to track it down on our reanimated hooks? Thanks!

@talaikis
Copy link

Same with RN 0.73.4. Had no such problem with RN 0.72 and 3.3.0. Resolution does nothing in prod.

@tomekzaw
Copy link
Member

tomekzaw commented Feb 18, 2024

Hey @efstathiosntonas, thanks for reporting this issue.

I don't know if this is related but while testing a PR #5216 I found that some AP initWithContext/NativeProxy I had to add a non optional parameter which is a valueUnpackerCode of type String. You may have to include this missing argument/type in some function call or provide its default value.

@joe-sam This API is private and used only internally, you should never need to make any changes to it.

You possibly may have multiple mismatched library versions that might be trying to invoke the reanimated API from the earlier/later library interface. I had to pin the resolutions for the peer dependency in my package.json before my incompatibility issues went away.

@joe-sam Having multiple versions of Reanimated installed at once usually breaks things. That's why in #4914 (released in 3.6.0) we added a runtime check that asserts that only one instance of Reanimated is present:

function assertSingleReanimatedInstance() {
if (
global._REANIMATED_VERSION_JS !== undefined &&
global._REANIMATED_VERSION_JS !== jsVersion
) {
throw new Error(
`[Reanimated] Another instance of Reanimated was detected.
See \`https://docs.swmansion.com/react-native-reanimated/docs/guides/troubleshooting#another-instance-of-reanimated-was-detected\` for more details. Previous: ${global._REANIMATED_VERSION_JS}, current: ${jsVersion}.`
);
}
}

For more details, see https://docs.swmansion.com/react-native-reanimated/docs/guides/troubleshooting/#multiple-versions-of-reanimated-were-detected.

We recommend the solution from #5660 (comment).

hey guys, care to explain why this might be throwing and if there's a way to track it down on our reanimated hooks?

Sure, the error comes from this else statement:

} else {
throw new Error('[Reanimated] Data type not recognized by value unpacker.');
}

When copying JS values from RN runtime to UI runtime (e.g. worklet closure), they are first serialized from JS values to C++ shareables (intermediate format, runtime-agnostic data structures) and then parsed from C++ shareables to JS values.

valueUnpacker is a function used to parse more complex objects like worklets or remote functions.

The error in the issue description states that valueUnpacker cannot re-create passed object. Without a repro, it's really difficult to tell what exactly goes wrong.

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Feb 18, 2024

thanks @tomekzaw for the detailed explanation.

It seems that we need to revisit our animated hooks and methods and change things around. This is not an issue with 3.6.2 so recent changes have caused this.

Question is, are you going to revisit this else/if function and improve it/fix it? If not, this is breaking the API on several npm modules with reanimated as a dep since according to my bugsnag reports there are 2 npm modules that cause this crash. It's really hard to reproduce though 🤷‍♂️

@tomekzaw
Copy link
Member

Honestly, I'd like to eliminate valueUnpacker as soon as possible. This function is implemented in JS and we need to pass its source code from JS to native side in installTurboModule which makes things overly complicated. Currently, it's used in three different places and we can decouple them:

  • ShareableWorklet::toJSValuevalueUnpacker creates and memoizes worklets
  • ShareableHandle::toJSValuevalueUnpacker memoizes handles
  • ShareableRemoteFunction::toJSValuevalueUnpacker adds logic for "Tried to synchronously call" error

I think I will submit a PR with a small refactor.

@tomekzaw
Copy link
Member

@efstathiosntonas As described above, valueUnpacker is responsible for converting worklets, remote functions and values with initializers. However, the provided error clearly states that it has been called for something else that does not fall into any of these categories. This means that even if we refactor each individual branch, the error is still likely to persist.

At this point, we need to know what object caused valueUnpacker to fail. However, you have mentioned that you cannot reproduce the issue locally. Is there any way we could at least see what object causes the error? This could be easily done by appending the error message with the string representation of the object to unpack:

  } else { 
-   throw new Error('[Reanimated] Data type not recognized by value unpacker.'); 
+   throw new Error('[Reanimated] Data type not recognized by value unpacker.' + ' String(objectToUnpack)); 
  } 

@alexstanbury
Copy link

alexstanbury commented Feb 21, 2024

I'm having the same issue and can recreate it approx 1 in 5 times in my app by following a certain sequence. With your code amendment String(objectToUnpack) I get [Object object] appended to the end, and with JSON.stringify(objectToUnpack, null, 2) I get {}, an empty object.

I'm using 3.7.0 and RN 0.73.4

This crash is happening on Android, interestingly I get a different one if I follow the same sequence on iOS. I will update that tomorrow.

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Feb 21, 2024

thanks @alexstanbury, @tomekzaw no matter how many times I try I cannot get it to crash in my environment, sorry 🫤

The part of the code where this crashes was working perfectly fine on < 3.7.0, it srarted showing up after the upgrade on my bugsnag logs.

edit: this module creates the crash: https://github.com/SimformSolutionsPvtLtd/react-native-reactions

@tomekzaw
Copy link
Member

@alexstanbury Can you try _toString(objectToUnpack) instead?

@joe-sam
Copy link

joe-sam commented Feb 22, 2024

The current stack trace is useless as when this error/exception occurs a redundant stack trace is printed.
My suggestion is since all supported versions of react-native can finally handle non-standard error objects (facebook/react-native#42079) perhaps a stack trace of the other thread could be injected into the wrapper object (objectToPack) before it is initially marshalled/serialized by the valuePacker. This can be useful device for debugging and it would at the very minimum show the file where the exception was actually thrown. You can delete this stacktrace(or remove the wrapper) if un-marshalled/deserialized successfully or enable this only during development.

@tomekzaw
Copy link
Member

tomekzaw commented Feb 22, 2024

@joe-sam Just to confirm that I understand your proposal correctly – your idea is to get the stack trace from the RN runtime when the value is serialized from JS value into a C++ intermediate form (technically, this is called makeShareableClone) and then include that string in the C++ serializable form, so that we can access it when parsing back from C++ to JS value in valueUnpacker?

This way we would at least know the origin of the values copied into the UI runtime, in terms of what functions passed them to makeShareableClone. On the other hand, we still wouldn't exactly know anything about the values itself (type, keys etc.).

@alexstanbury
Copy link

I'll be testing & recreating this morning so if you can provide instructions on how to make the above changes, I'll give it a go.

@dpyeates
Copy link

I am seeing this in production as well. RN0.73.4, Reanimated 3.7.0.

Screenshot 2024-02-22 at 08 03 37

@tomekzaw
Copy link
Member

@dpyeates Thanks for letting us know!

@alexstanbury Great to hear, thanks in advance! Let's start with the following change that should print more information about the failing value:

  } else { 
-   throw new Error('[Reanimated] Data type not recognized by value unpacker.'); 
+   throw new Error('[Reanimated] Data type not recognized by value unpacker.' + ' _toString(objectToUnpack)); 
  } 

@SukumarRN7
Copy link

image

Happening on

  • "react-native": "^0.72.0",
  • "react-native-reanimated": "^3.6.1",

This was working fine with the same above versions since 2 months i guess,
and suddenly it started coming up since yesterday and frequency is quiet high as well.

@joe-sam
Copy link

joe-sam commented Feb 22, 2024

@tomekzaw Yes, essentially correct (and could be hidden behind a feature flag), so as to easily create a reproducer which is only possible if we know the context of the exception.

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Feb 29, 2024

@tjzel don't know if it makes any sense but iOS is in 2000 3000 renders (17 25minutes) using this neat polyfill console.count() https://github.com/MaxGraey/react-native-console-time-polyfill and it still hasn't crashed while on Android it crashed in the first 2 minutes.

edit: the app won't crash on simulator, just the red screen of death, the counter is keep going though on the background.

simulator screenshot:

Click me

Screenshot 2024-02-29 at 15 27 17

@Elrohil44
Copy link

Elrohil44 commented Feb 29, 2024

Well, for me it also happens randomly. Sometimes it happens every 5 minutes and sometimes it is running for several minutes without error. In my case, I am testing it on Android, so maybe indeed Android has a higher chance to trigger error.

Edit: That is correct, there is an error on the red screen with this stacktrace, but you can dismiss it and the app still works

@SukumarRN7
Copy link

@tjzel this is also happening on IOS.

in production, although it didnot print any stack trace.

image

@tsogzark
Copy link

tsogzark commented Mar 2, 2024

I have encountered the same stack trace. This issue manifests on my Android and iPad devices with an approximate probability of 1 in 50.Thanks to guys who are working on the issue.

version: 3.7.2

@aspidvip
Copy link

aspidvip commented Mar 4, 2024

me to

@tjzel
Copy link
Collaborator

tjzel commented Mar 4, 2024

Thanks everyone for your effort here. Thanks to reproduction postem by @Elrohil44 we managed to find the underlying problem and it's a race condition. We will soon patch this up and try to release Reanimated 3.8.0 shortly afterwards.

@Latropos Latropos added Repro provided A reproduction with a snippet of code, snack or repo is provided and removed Unable to repro Missing repro This issue need minimum repro scenario labels Mar 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
…ds (#5759)

## Summary

Fixes #5660, a regression introduced in #4300.

The aforementioned race condition happens this way:

1. An object using a `ShareableHandle` underneath (e.g. a shared value)
is created.
2. This object is accessed on UI thread.
3. The initializer of this `ShareableHandle` get called on the UI
thread.
4. At the same time, JS thread schedules an operation on this object
(e.g. setting a value of shared value).
5. Access on UI thread forces the initialization. The condition of the
if clause resolves to true and UI thread tries to access the runtime.
![Screenshot 2024-03-04 at 13 00
07](https://github.com/software-mansion/react-native-reanimated/assets/40713406/6c124599-3d60-4211-a1a9-3ccedd88f687)
6. However, access from JS thread has locked the runtime and causes the
UI runtime to wait.
7. Then, JS thread enters the same if clause body and initializes the
whole shareable.
8. After initializing and releasing the runtime, UI thread gets
unblocked.
9. However, now `initializer_` has been nulled and causes memory access
issues.

It's difficult to change the whole flow of locking to prevent such
scenarios. Therefore we won't null the `initializer_` object anymore.
However, this won't fix the problem of potential double initialization.
Luckily, the code of `valueUnpacker` already prevents that with its
shareable handle cache and the fact that runtime operations must be
sequential.

## Test plan

Run the following race condition reproduction made - you should re-run
the app several times (probably up to 20ish) since it's most likely to
happen when the app starts. It's also more likely to happen on Android
simulators (at least for me).

<details>
<summary>
Test code
</summary>

```tsx
import {useSharedValue} from 'react-native-reanimated';
import {useEffect, useState} from 'react';

const value = 666666;

const Screen = () => {
  const sv = useSharedValue(value);
  useSharedValue(1);
  useSharedValue(2);
  useSharedValue(3);
  sv.value = value;

  useEffect(() => {
    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    const currentValue = sv.value;
  }, [sv]);

  return null;
};
const ReanimatedCrashReproduction = () => {
  const [render, setRender] = useState(false);

  useEffect(() => {
    const interval = setInterval(() => setRender(r => !r), 500);
    return () => clearInterval(interval);
  }, []);

  return render ? <Screen /> : null;
};

export default ReanimatedCrashReproduction;
```

</summary>

You can do the following to see that double initialization happens
still, but all is well 🚰.

![Screenshot 2024-03-06 at 10 06
50](https://github.com/software-mansion/react-native-reanimated/assets/40713406/6517cd3a-eea0-45ef-bd18-a15215272f13)

Big thanks to @piaskowyk for the debugging help 🚀
@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Mar 6, 2024

@tjzel thank you for fixing this 🔥 , is there an ETA for 3.8.0 😅 ?

@tjzel
Copy link
Collaborator

tjzel commented Mar 6, 2024

image

@tjzel
Copy link
Collaborator

tjzel commented Mar 6, 2024

It's an absolute priority for us to ship production crash fixes asap 🚤

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Mar 6, 2024

finally I'll get rid of 1K lines of my monkey(donkey-dog-cat)-patch of reanimated

@angelica-snowit
Copy link

still waiting a patch for this :'(

@tomekzaw
Copy link
Member

tomekzaw commented Mar 8, 2024

@angelica-snowit This issue was fixed with #5759 which has already been merged to main and thus should be also released as nightly on npm. Please try yarn add react-native-reanimated@nightly.

@tmitchel2
Copy link

I downgraded to 3.6.3 but im still getting this issue. Is that expected and is it known which version is the last known good version?

@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Mar 8, 2024

@tmitchel2 this started after upgrading to 3.7.0, never encountered it on 3.6.2, it shouldn't happen in 3.6.3 though since it only included some web fixes in comparison to 3.6.2 according to the release notes.

Did you cleaned your project and metro cache after downgrading?

@SukumarRN7
Copy link

SukumarRN7 commented Mar 8, 2024

@tmitchel2 Can you give a look on ur npm/yarn lock file

  • check for if reanimated is pointing to 3.6.2 but is still referring to use 3.7.0 because of "^" sign.
    You might need to change that explicitly to use 3.6.2 .

This can be the culprit !!
image

@tmitchel2
Copy link

I had the exact same thought and when I downgraded again I locked it down to 3.6.1, but actually looking back in git I can see it was indeed still using 3.7.1 in the lockfile

(facepalm)

:)

@efstathiosntonas
Copy link
Contributor Author

3.8.0 is out, thanks @tjzel ❤️

@tomgreco
Copy link

tomgreco commented Apr 16, 2024

I'm getting this error on 3.8.1 with jest.

 node_modules/react-native-reanimated/src/reanimated2/valueUnpacker.ts:67:94 - error TS2552: Cannot find name '_toString'. Did you mean 'toString'?

    67       `[Reanimated] Data type in category "${category}" not recognized by value unpacker: "${_toString(
                                                                                                    ~~~~~~~~~

      node_modules/typescript/lib/lib.dom.d.ts:18181:18
        18181 declare function toString(): string;

@andrecrimb
Copy link

I'm still getting this in 3.8.1.

@quantumperception
Copy link

quantumperception commented May 28, 2024

I was getting this in Jest tests. I was mocking using react-native-reanimated/mock, solved it by using jest.mock('react-native-reanimated') instead.

RN 0.74.1, Expo SDK 51, Reanimated 3.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided
Projects
None yet