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

Error "Malformed calls from JS: field sizes are different" (caused by "Cannot freeze") occurs on 0.60.5 #27

Closed
mcuelenaere opened this issue Oct 17, 2019 · 12 comments · Fixed by Kudo/v8-android-buildscripts#5

Comments

@mcuelenaere
Copy link

When running RN 0.60.5 with V8, in development mode the "Malformed calls from JS: field sizes are different" error gets triggered for our code.

When investigating, this seems to be caused by JS trying to freeze the params that are being passed to the bridge at https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/MessageQueue.js#L313.

Object.freeze() throws an error with the unhelpful message "Cannot freeze".
When JSONifying the particular object that is being froze, this is what I see: [{"size":228,"offset":0,"blobId":"f7ece23d-646d-4e09-ba54-0e855932ed26","__collector":{}},"UTF-8",22900,22901].

My hypothesis is that V8 is somehow unable to freeze this particular blob object, while this was possible in JSC/Hermes.

I'll workaround the issue by catching the error and ignoring it (since this is only trying to help the developer and not a strict requirement), but this information might be useful to others and/or this issue might want to be looked into.

@mcuelenaere
Copy link
Author

This is the diff we're using in our react-native fork:

diff --git a/Libraries/BatchedBridge/MessageQueue.js b/Libraries/BatchedBridge/MessageQueue.js
index 8fce2a7e67..afbf514103 100644
--- a/Libraries/BatchedBridge/MessageQueue.js
+++ b/Libraries/BatchedBridge/MessageQueue.js
@@ -284,8 +284,14 @@ class MessageQueue {
         JSON.stringify(params, replacer),
       );

-      // The params object should not be mutated after being queued
-      deepFreezeAndThrowOnMutationInDev((params: any));
+      try {
+        // The params object should not be mutated after being queued
+        deepFreezeAndThrowOnMutationInDev((params: any));
+      } catch (err) {
+        // This is not a blocking issue, show a warning but continue executing
+        // (see https://github.com/Kudo/react-native-v8/issues/27 for more context)
+        console.warn('Cannot freeze: ' + JSON.stringify(params, replacer));
+      }
     }
     this._queue[PARAMS].push(params);

mcuelenaere added a commit to getdelta/react-native that referenced this issue Oct 17, 2019
…appening on V8.

This is caused by an incompatibility between V8's Object.freeze() functionality and
the react-native blob module. Since the deepFreeze function in this instance is only
a developer utility and not a requirement, this can safely be silenced.

See Kudo/react-native-v8#27 for more context
@Kudo
Copy link
Owner

Kudo commented Oct 19, 2019

Thanks for the briefly information, I will try to find the root cause accordingly.

@Kudo
Copy link
Owner

Kudo commented Oct 19, 2019

@mcuelenaere Do you mind to provide blob data to be Object.freeze()?
You could try to add `console.log' in MessageQueue.js.

@mcuelenaere
Copy link
Author

mcuelenaere commented Oct 21, 2019

@Kudo

this is the stacktrace:

TypeError: Cannot freeze
    at Function.freeze (<anonymous>)
    at deepFreezeAndThrowOnMutationInDev (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:2460:14)
    at deepFreezeAndThrowOnMutationInDev (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:2467:11)
    at deepFreezeAndThrowOnMutationInDev (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:2467:11)
    at MessageQueue.enqueueNativeCall (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:2116:11)
    at http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:1688:25
    at tryCallTwo (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:24504:7)
    at doResolve (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:24668:15)
    at new Promise (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:24527:5)
    at Object.fn [as readAsText] (http://localhost:8081/index.bundle?platform=android&dev=true&minify=false:1687:16)

and these are JSON.stringify() of some blobs:

[{"size":228,"offset":0,"blobId":"a48e5968-11a3-4692-a36c-746a383a4216","__collector":{}},"UTF-8",25890,25891]
[{"size":3340,"offset":0,"blobId":"6aa40fc3-5415-48cf-9d6b-1aab4f614edb","__collector":{}},"UTF-8",44036,44037]
[{"size":228,"offset":0,"blobId":"a48e5968-11a3-4692-a36c-746a383a4216","__collector":{}},"UTF-8",25890,25891]

My hypothesis is that the __collector member variable is the issue, which is probably a reference to a native object created at https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/modules/blob/jni/BlobCollector.cpp#L54

@Kudo
Copy link
Owner

Kudo commented Oct 23, 2019

@mcuelenaere Could you double confirm react-native version is 0.60.5 and react-native-v8 version is 0.60.5-patch.0?
As the BlobCollector did not land on RN 0.60 yet, the code path should not hit on the BlobCollector you mentioned.
One possibility is that you use wrong react-native-v8 version, e.g. 0.61.1-patch.0.

For the V8 problem, V8 cannot do Object.freeze on object with interceptors.
react-native-v8 did add interceptors on RN JSI hosted object.
I will test Blob on RN 0.61 anyway and see if I could overcome the issue.
Thanks for your information.

@mcuelenaere
Copy link
Author

@Kudo yes, that's because we use a fork of 0.60.5 with some fixes from 0.61 cherry picked on top (see https://github.com/getdelta/react-native/commits/0.60-patched%2Bv8 )

@Kudo
Copy link
Owner

Kudo commented Oct 23, 2019

@mcuelenaere Thanks, I was able to reproduce the problem from your forked and the code.

const TestCase = async () => {
  const resp = await fetch('http://www.africau.edu/images/default/sample.pdf', {
    method: 'GET',
  });
  const respBlob = await resp.blob();
  await AsyncStorage.setItem('foo', respBlob);
  console.log('respBlob', respBlob);
};

Passing an JSI hosted object through RN bridge will cause the problem.

Unfortunately, I have no proper solution in the mean time.
That is V8's design not to support Object.freeze for objects with interceptors and it is somehow right.
Please let me think a proper idea for a while.

Thank you.

@Kudo
Copy link
Owner

Kudo commented Oct 26, 2019

@mcuelenaere I've sent a PR to address the issue.
facebook/react-native#27013
Since you have forked RN, you may able to apply the patch first in your repo and see if the fix helps.

@winseyli
Copy link

Hi there. I have the same problem with RN version 0.61.4 (react-native-v8 version 0.61.4-patch.1)
Issue can be reproduced by response.json() after fetch

@Kudo
Copy link
Owner

Kudo commented Dec 17, 2019

@mcuelenaere @winseyli Considering the PR not updated for a while, I finally decide to workaround the issue in V8.
Please try to use v8-android@7.8.2 / v8-android-nointl@7.8.2 and see if this could solve your problems. Thank you.

@winseyli
Copy link

@Kudo Thanks it works 👍

@gmaclennan
Copy link

Thanks for taking the time to fix this @Kudo, the update works and our app is working again!

svenlombaert pushed a commit to getdelta/react-native that referenced this issue Jan 7, 2020
…appening on V8.

This is caused by an incompatibility between V8's Object.freeze() functionality and
the react-native blob module. Since the deepFreeze function in this instance is only
a developer utility and not a requirement, this can safely be silenced.

See Kudo/react-native-v8#27 for more context
svenlombaert pushed a commit to getdelta/react-native that referenced this issue Jan 7, 2020
…appening on V8.

This is caused by an incompatibility between V8's Object.freeze() functionality and
the react-native blob module. Since the deepFreeze function in this instance is only
a developer utility and not a requirement, this can safely be silenced.

See Kudo/react-native-v8#27 for more context
svenlombaert pushed a commit to getdelta/react-native that referenced this issue Jan 7, 2020
…appening on V8.

This is caused by an incompatibility between V8's Object.freeze() functionality and
the react-native blob module. Since the deepFreeze function in this instance is only
a developer utility and not a requirement, this can safely be silenced.

See Kudo/react-native-v8#27 for more context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants