bug: MessageHandler usage of JavaScriptReplyProxy is triggering a native crash on webview

[Charchad]

Bug Report

Capacitor Version

@capacitor/cli: 4.1.0
@capacitor/android: 4.1.0
@capacitor/ios: 4.1.0
@capacitor/core: 4.1.0

Platform(s)

Android

Current Behavior

For some reason the app crashes when the webview is under stress, triggering a native crash:

Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0 in tid 13176 (app package id), pid 13176 (app package id)

*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  Build fingerprint: 'google/crosshatch/crosshatch:12/SP1A.210812.016.C1/8029091:user/release-keys'
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  Revision: 'MP1.0'
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  ABI: 'arm64'
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  Timestamp: 2022-09-23 16:34:21.107842893+0200
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  Process uptime: 0s
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  Cmdline: app package id
2022-09-23 16:34:21.809 13609-13609 DEBUG                   pid-13609                            A  pid: 13176, tid: 13176, name: app process name >>> app package id <<<
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A  uid: 11403
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A  signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A  Cause: null pointer dereference
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x0  0000007fd1c72788  x1  0000007fd1c72788  x2  0000007e55faf2c0  x3  0000007ce48c55dc
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x4  0000007fd1c723f0  x5  0000007fd1c72308  x6  0000000000000000  x7  0000000000000000
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x8  0000000000000000  x9  3192123bb699877f  x10 0000000000000010  x11 0000000000000160
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x12 000000000000000a  x13 0000000000000000  x14 0000007fd1c71c20  x15 000000000000000f
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x16 0000000000000001  x17 0000007f981e6b40  x18 0000007f9ebd2000  x19 0000000000000000
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x20 0000007da5fa9ab0  x21 0000007a005c70c0  x22 0000007da5fa9ab0  x23 0000000000000007
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x24 0000000000000007  x25 0000007fd1c728a8  x26 0000000010300011  x27 0000000000000008
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      x28 0000007fd1c728c0  x29 0000007fd1c727a0
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A      lr  0000007c7fe4ccbc  sp  0000007fd1c72780  pc  0000007c7fe4ccbc  pst 0000000060000000
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A  backtrace:
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A        #00 pc 0000000001c28cbc  /data/app/~~bJpCPv5dhtSWFX32lsHHfg==/com.google.android.webview-tjDobPBo0L-9pipJRhM6aQ==/base.apk!libmonochrome.so (Java_J_N_MayS5i9E+80) (BuildId: 30bb452c0c8c3eb67961996b4977370b3485b3d3)
2022-09-23 16:34:21.810 13609-13609 DEBUG                   pid-13609                            A        #01 pc 00000000001e63f4  /data/app/~~bJpCPv5dhtSWFX32lsHHfg==/com.google.android.webview-tjDobPBo0L-9pipJRhM6aQ==/oat/arm64/base.odex (art_jni_trampoline+132)
2022-09-23 16:34:21.840   827-827   tombstoned              pid-827                              E  Tombstone written to: tombstone_17

I have changed the app package id for security reasons.

Expected Behavior

The app shouldn't crash

Code Reproduction

The crash is not triggered anymore if the change that was made on sendResponseMessage is reverted:

Now (Doesn't Work):

boolean isValidCallbackId = !call.getCallbackId().equals(PluginCall.CALLBACK_ID_DANGLING);
if (isValidCallbackId) {
   if (WebViewFeature.isFeatureSupported(WebViewFeature.WEB_MESSAGE_LISTENER) && javaScriptReplyProxy != null) {
       javaScriptReplyProxy.postMessage(data.toString());
   } else {
       final String runScript = "window.Capacitor.fromNative(" + data.toString() + ")";
       final WebView webView = this.webView;
       webView.post(() -> webView.evaluateJavascript(runScript, null));
    }
}

Before (Works):

boolean isValidCallbackId = !call.getCallbackId().equals(PluginCall.CALLBACK_ID_DANGLING);
if (isValidCallbackId) {
   final String runScript = "window.Capacitor.fromNative(" + data.toString() + ")";
   final WebView webView = this.webView;
   webView.post(() -> webView.evaluateJavascript(runScript, null));
} else {
   bridge.getApp().fireRestoredResult(data);
}
              

So I guess something is wrong with this call:

javaScriptReplyProxy.postMessage(data.toString());

Other Technical Details

The crash doesn't happen with Capacitor 3.8.0 but still happens with Capacitor 4.3.0

npm --version output:
8.1.0

node --version output:
v16.13.0

[Ionitron]

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks!
Ionitron 💙

[Charchad]

@jcesarmobile, sorry for my late reply. Is really hard for us to provide a sample app as there is a proprietary login SDK also involved and we cannot share this.

In our team, we think the problem might be related to that when a user is logging in / out, this login SDK opens another WebView. After the process is finished, a capacitor plugin notifies the web content (hosted on the main WebView) that the page needs to be reloaded.

Is there any other way to proceed with this issue?

[jcesarmobile]

There is no way to proceed if we can't reproduce.
Since your SDK is involved, it might be a problem on your SDK.

[Charchad]

But the crash doesn't happen on the login SDK, but on the main WebView.
Also as I said before If we revert this change that was introduced on MessageHandler in Capacitor 4.1.0, the crash is gone.

[jcesarmobile]

then you can provide a sample app where the crash still happens but the SDK is not involved.

[Charchad]

I have an idea, would it be possible to make the above change something that can be turned on / off by a new flag in CapConfig?

[jcesarmobile]

I've added this as feature request to allow to use the old bridge.
But there are a lot more chances of working on it if it was a bug that could be reproduced instead of a feature request.

Also using the old bridge is more insecure and google could remove it at any time while the new one is more future proof.

[hermitdemschoenenleben]

I see the same problem in production, it appeared after upgrading to capacitor 4. However, I'm also not able to reproduce the issue as I have no idea under which circumstances it appears (unfortunately for this issue Sentry error logs don't contain any information on what happened before the crash).
@Charchad did you experience any negative effects of reverting the code in sendResponseMessage? Being not future proof is not my main concern at the moment :)

[Charchad]

Hi @hermitdemschoenenleben,
We didn't notice any problems after reverting the change but I might be wrong, but of course we can't fix this on our side as this class in part of the code that is automatically added by Capacitor into the Android project.

[peitschie]

May or may not be relevant here... but I get a crash like this when using a plugin that needs a dangerous Android permission, and attempting to invoke this immediately during start up.

In my specific case, I'm using cordova-plugin-ble-central, and invoking a scan on app launch. A scan requires the user to allow location permissions via a system dialogue. I found that I had to put a small delay (e.g. 500ms) before starting the scan, as otherwise the permission request gets stuck in an infinite loop.

In my case, it's because the plugin permissions response gets called with an empty permissions collection that has no granted or denied permissions, which causes the ble plugin to immediately make another permissions request. The infinite recursion eventually crashes the browser in a second or two during launch.

Apologies if this is unrelated to the issue here... Just adding the comment in case it's a useful hint.

[hermitdemschoenenleben]

@Charchad ok, thanks a lot! I'll try it out then by forking capacitor and replacing these lines.

@peitschie thanks a lot for sharing your insight! At least for my app, this doesn't seem to be the cause of the problem though as I don't request dangerous android permissions on app start

[Charchad]

Is there any update of when this new flag to toggle between the old/new bridge will be added?

@hermitdemschoenenleben, forking Capacitor solved your crashes?

[jcesarmobile]

There has been another similar issue reported and the problem there is that the new bridge runs plugins in main thread while the old bridge uses a background thread.
Can you check if making this change fixes your issue?
in this line

capacitor/android/capacitor/src/main/java/com/getcapacitor/MessageHandler.java

Line 30 in f3ab951

postMessage(message.getData());

bridge.execute(() -> {
    postMessage(message.getData());
});

[Charchad]

Hi @jcesarmobile, I just tried your suggested change but I'm afraid the crash still happens.

[hermitdemschoenenleben]

@Charchad I'm rolling out a new version that includes the fix of your first post and this seems to fix the crash. As we have a tight schedule right now, I won't be able to test jcesarmobile's fix for the next few weeks, though.

[ionitron-bot]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.