Use ConcurrentHashMap for handling concurrent Android websockets, and…

[sunweiyang]

… prevent unknown websocket IDs from crashing on Android (show warning on development builds instead)

Motivation

This PR addresses #3346; an unknown websocket ID should produce a warning during development, but not cause crashes in production RN apps. This PR was created by @satya164's request, and was inspired by @tanthanh289's suggestion on #3346's thread.

Test Plan

On Android, create a websocket using a service like Pusher (pusher-js npm package) or manually, and then induce removal of its websocket ID. Result should be a red warning screen during development, and no crash in the app's release variant.

Release Notes

[ANDROID] [BUGFIX] [WebSocket] - Prevent unknown websocket IDs from crashing on Android

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[sunweiyang]

I've completed the CLA on https://code.facebook.com/cla, as requested by facebook-github-bot. Please let me know if any other action is necessary. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[satya164]

cc @hramos

Would be great to get this landed

[hramos]

@sunweiyang please rebase and I can look into getting this imported. Thanks!

[sunweiyang]

@hramos, rebased as requested.

[yewenxiang23]

I modified the code according to your PR, and the first break will not cause a crash, but the second one is the same error.

[sunweiyang]

@yewenxiang23, the following RuntimeException have been removed whenever a websocket ID is unknown, so I'm not sure what is causing your error:

throw new RuntimeException("Cannot send a message. Unknown WebSocket id " + id);		

Can you check your app's logcat to see where the failure occurred, and if it's related to this PR's changes to WebSocketModule.java (or somewhere else)?

[facebook-github-bot]

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[yewenxiang23]

@sunweiyang

--------- beginning of crash
--------- beginning of system
--------- beginning of main
02-12 17:41:52.554 28585 28585 D ReactNative: ReactInstanceManager.onReloadWithJSDebugger()
02-12 17:41:52.554 28585 28585 D ReactNative: ReactInstanceManager.recreateReactContextInBackground()
02-12 17:41:52.554 28585 28585 D ReactNative: ReactInstanceManager.runCreateReactContextOnNewThread()
02-12 17:41:52.569 28585 28585 D ReactNative: ReactInstanceManager.tearDownReactContext()
02-12 17:41:52.608 28585 28585 D ReactNative: CatalystInstanceImpl.destroy() start
02-12 17:41:52.664 28585 30660 D ReactNative: ReactInstanceManager.createReactContext()
02-12 17:41:52.870 28585 30660 D ReactNative: Initializing React Xplat Bridge.
02-12 17:41:52.874 28585 30660 D ReactNative: Initializing React Xplat Bridge before initializeBridge
02-12 17:41:52.878 28585 30660 D ReactNative: Initializing React Xplat Bridge after initializeBridge
02-12 17:41:52.878 28585 30660 D ReactNative: CatalystInstanceImpl.runJSBundle()
02-12 17:41:52.878 28585 30665 D ReactNative: ReactInstanceManager.setupReactContext()
02-12 17:41:52.879 28585 30665 D ReactNative: CatalystInstanceImpl.initialize()
02-12 17:41:52.879 28585 30665 D ReactNative: ReactInstanceManager.attachRootViewToInstance()
02-12 17:42:45.117 30821 30821 D ReactNative: ReactInstanceManager.ctor()
02-12 17:42:45.177 30821 30821 D ReactNative: ReactInstanceManager.createReactContextInBackground()
02-12 17:42:45.177 30821 30821 D ReactNative: ReactInstanceManager.recreateReactContextInBackgroundInner()
02-12 17:42:45.499 30821 30821 D ReactNative: ReactInstanceManager.onReloadWithJSDebugger()
02-12 17:42:45.500 30821 30821 D ReactNative: ReactInstanceManager.recreateReactContextInBackground()
02-12 17:42:45.500 30821 30821 D ReactNative: ReactInstanceManager.runCreateReactContextOnNewThread()
02-12 17:42:45.616 30821 30877 D ReactNative: ReactInstanceManager.createReactContext()
02-12 17:42:45.800 30821 30877 D ReactNative: Initializing React Xplat Bridge.
02-12 17:42:45.803 30821 30877 D ReactNative: Initializing React Xplat Bridge before initializeBridge
02-12 17:42:45.806 30821 30877 D ReactNative: Initializing React Xplat Bridge after initializeBridge
02-12 17:42:45.806 30821 30877 D ReactNative: CatalystInstanceImpl.runJSBundle()
02-12 17:42:45.807 30821 30886 D ReactNative: ReactInstanceManager.setupReactContext()
02-12 17:42:45.808 30821 30886 D ReactNative: CatalystInstanceImpl.initialize()
02-12 17:42:45.809 30821 30886 D ReactNative: ReactInstanceManager.attachRootViewToInstance()
02-12 17:43:36.230 30821 30821 D ReactNative: CatalystInstanceImpl.destroy() start
02-12 17:43:36.234 30821 30884 D ReactNative: CatalystInstanceImpl.destroy() end

react-native v0.53.0

[satya164]

@yewenxiang23 from the error message it looks like you are using the old code since this error message is removed in the PR. are you compiling from source?

[yewenxiang23]

@satya164 Sorry I'm a little don't understand you mean, I have updated the version the react - native 0.53.0, and found the android when websocket disconnect the second reconnection, or there will be a flash back, I according to the modified the WebSocketModule PR file, will appear this problem, the IOS above is not the problem.

[satya164]

@yewenxiang23 modifying the files is not enough. you also need to compile from source https://facebook.github.io/react-native/docs/android-building-from-source.html

[yewenxiang23]

@satya164 Thank you very much for your answer. I will try it now.