Fix race conditions for Android Support SDK 27

[kristfal]

We'll deploy to production this Wednesday. I propose you wait for us to get analytics from users before we merge. Fixes #42.

[kristfal]

So the update is live, and we're still seeing crashes, but at a new location in the function. @cesardeazevedo, this is the new crash stack:

java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.Object java.lang.ref.WeakReference.get()' on a null object reference

com.bottomsheetbehavior.RNBottomSheetBehavior.onInterceptTouchEvent RNBottomSheetBehavior.java:267

android.support.design.widget.CoordinatorLayout.resetTouchBehaviors CoordinatorLayout.java:394
android.support.design.widget.CoordinatorLayout.onInterceptTouchEvent CoordinatorLayout.java:509
android.view.ViewGroup.dispatchTouchEvent ViewGroup.java:2504

Which is here.

I'm wondering if we should do something like:

if ( mViewDragHelper == null ) {
      return false;
}

At the top of the function instead to exit early instead. Do you think this will have any negative side effects beyond "ignoring" the first touch before everything is set up properly?

[cesardeazevedo]

Thanks for the report, i am going to take a look more closely this weekend, i am wondering if a full rebase of the BottomSheetBehavior from the SDK 27 is needed, since i have implemented the anchor point feature, the source had to be copied, and was likely copied from the SDK 25 instead.

[kristfal]

Thanks, that may be a solution. In the short term, do you see any clear issues with returning false? We’re aiming to get a fix out to prod as soon as possible.

[cesardeazevedo]

I am not sure about returning false, i need more tests before give you such claim, the hardest part is to get a way to reproduce the problem, if you could help me with it, i would really appreciate.

[kristfal]

The race condition happens very rarely (some 120 crashes per week amongst 30k active units with this component), so its very hard to tell what happens and how. I'm guessing there may be a touch event triggered during init of the component, but its hard to tell.

We'll try returning false as an interim stopgap regardless. I'll let you know how it works out.

[cesardeazevedo]

I am really happy to know that this component has been used quite often, let me know about the results, and if there's anything that we can do to overcome it.

[dmkmedina]

Hey @kristfal,

I'm curious if adding the null check for mViewDragHelper helped resolve the issue for you. We're facing similar issues on our end.

[kristfal]

@dmkmedina I don't remember the specifics, but I am afraid the Java solution wasn't adequate. We ended up deferring loading TouchThroughWrapper until later in the app's lifecycle from the JS side. Haven't seen any crashes since.