Skip to content

Commit bb1bba0

Browse files
agrawal-dgnprice
authored andcommitted
sharing: Reset initialSharedData after sending the map to JS.
This resolves the 'Map already consumed' bug. `Map already Consumed` is a fatal JS exception that occurs when react bridge gives `ObjectAlreadyConsumedException`. This exception according to the react-native documentation [1] at the time of writing this commit is given when: "caller attempts to modify or use a `WritableNativeArray` or `WritableNativeMap` after it has already been added to a parent array or map. This is unsafe since we reuse the native memory so the underlying array/map is no longer valid." One possible way to reproduce this issue is: - revert changes made in this commit (comment out line `initialSharedData = null` in `android/**/sharing/SharingModule.kt`) - start the app using `tools/run-android` - kill the app - start another app and share text to zulip (It is important that zulip starts using share intent, hence we do this step. It is also important to share text, data with content doesn't leave the app in recent apps view (discussed in next step), share playstore links as an example.) - cancle the share and quit zulip (use back presses if necessary), make sure to "quit" the app and not "kill" the app. - notice the app is still in the recent apps view (accessible by long pressing home) - start the app from recent apps. - map already consumed should popup. In this scenario it is assumed that when we backpress to quit zulip, android system does not flush the entire memory of the app (so that it can boot up faster, which is also noticable here). This also involves the static variable `initialSharedIntent` we set when the app was started previously using share intent. But it seems that react native discovers that the data in initialSharedIntent is reused and hence is unsafe to use thereby giving us the exception and the bug. The change introduced in this commit will fix any `map already consumed` issue concerning `initialSharedIntent`, since we reset its value just after its use. A more specific place to reset initialSharedData is within `MainActivity.kt` inside condition that checks for `FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY`. This will only resolve the steps of reproduction discussed above, but the error may pop up somewhere else. [1] https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/ObjectAlreadyConsumedException.java Commit message authored-by: Akash Dhiman <[email protected]>
1 parent d99ad3a commit bb1bba0

File tree

2 files changed

+4
-3
lines changed

2 files changed

+4
-3
lines changed

android/app/src/main/java/com/zulipmobile/sharing/SharingModule.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ internal class SharingModule(reactContext: ReactApplicationContext)
1010
}
1111

1212
@ReactMethod
13-
fun getInitialSharedContent(promise: Promise) {
13+
fun readInitialSharedContent(promise: Promise) {
1414
promise.resolve(initialSharedData)
15+
initialSharedData = null
1516
}
1617

1718
companion object {

src/sharing/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { Dispatch, SharedData, GetState } from '../types';
66
import { navigateToSharing } from '../actions';
77

88
const Sharing = NativeModules.Sharing ?? {
9-
getInitialSharedContent: () =>
9+
readInitialSharedContent: () =>
1010
// TODO: Implement on iOS.
1111
null,
1212
};
@@ -16,7 +16,7 @@ const goToSharing = (data: SharedData) => (dispatch: Dispatch, getState: GetStat
1616
};
1717

1818
export const handleInitialShare = async (dispatch: Dispatch) => {
19-
const initialSharedData: SharedData | null = await Sharing.getInitialSharedContent();
19+
const initialSharedData: SharedData | null = await Sharing.readInitialSharedContent();
2020
if (initialSharedData !== null) {
2121
dispatch(goToSharing(initialSharedData));
2222
}

0 commit comments

Comments
 (0)