Suspending gesture detecting functions should be combinable [181577176]

WAI

Bug

Status Update

No update yet.

Description

da...@danielzfranklin.org

created issue #1

Mar 1, 2021 08:15PM

I had code that required detecting a number of different gestures that were possible only in certain states. I started off using the gesture detecting functions and global variables like so:

var isInTurn = false
var isInSelect = false

detectDragGesturesAfterLongPress { change, _ ->
    if (!isInTurn) {
        isInSelect = true
        /* ... */
    }
}

detectDragGestures { change, _ ->
    if (!isInSelect) {
        isInTurn = true
        /* ... */
    }
}
// ...

I eventually traced a number of bugs down to updating the state non-atomically, so I was planning to add a mutex. That reminded me of how coroutines can often be used to solve concurrency issues, and I came up with

while (true) {
    val event = awaitPointerEventScope { awaitFirstDown() }

    when {
        isStartOfTurn(event) ->
            awaitTurnOrCancel(event)

        isStartOfSelect(event) ->
            awaitSelectOrCancel(event)

        // ...
    }
}

I defined a bunch of my own functions on PointerInputScope, and the approach solved my concurrency issues in a relatively clean way.

The problem is that most of the framework detectors block forever, so they can't be combined. I ended up copying a framework function, editing it to remove the line forEachGesture {, and copying over all their private/internal dependencies.

I'm suggesting that framework gesture detector functions should (when it makes sense) delegate to another public function that detects only one gesture.

fun detectDragGesturesAfterLongPress(/* ... */) {
   forEachGesture {
      detectOneDragGestureAfterLongPress(/* ... */)
    }
}

fun detectOneDragGestureAfterLongPress(/* ... */)

I'm also suggesting that more of the private helpers you use in gesture detection be made public, such as awaitLongPressOrCancellation, to enable users to write code that combines small suspending functions to detect complex gestures as cleanly as the framework.

Comments

ma...@google.com <ma...@google.com> #2Mar 2, 2021 02:33PM

Status: Won't Fix (Intended Behavior)

A couple of questions:

1. Have you saw crash in real device or only in simulators?
2. Do you use dynamic feature for language ID?

da...@danielzfranklin.org <da...@danielzfranklin.org> #3Mar 2, 2021 04:07PM

Tested on Android 12 Emulator with custom executor, but cannot repro this issue.

ma...@google.com <ma...@google.com> #4Mar 3, 2021 04:07PM

Second crash in the description is from a real device. Experienced it myself on two different Xiaomi phones, plus lots of crashes from users in the Google Play console.
Dynamic features are not used in the application.

As a wild guess, I have downgraded build tools from 31.0.0 to 30.0.3, compileSdk from 31 to 30, and moved all work with Language ID to the service in a separate process (just to be sure that crash can kill secondary process instead of main). This combination is in beta for 2 days by now and I don't see any SIGSEGV crashes.

ap...@google.com <ap...@google.com> #5Aug 12, 2022 06:10PM

Hmm, I feel the crash might be something related to separate/secondary process.

I also changed compileSdk and targetSDK to 31 but still cannot repro this issue.

je...@google.com <je...@google.com> #6Aug 12, 2022 07:45PM

On the contrary, there was no separate process before, when crashes started.

In the new build (with the aforementioned changes) I can see SIGSEGV crash, but only one instead of dozens and it has a bit different backtrace:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR)
liblanguage_id_jni.so (offset 0x11e000)

backtrace:
  #00  pc 000000000003c7c0  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003b960  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003bb48  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003bafc  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000036c98  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000032714  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000031cac  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000057438  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/oat/arm64/base.odex (offset 0x57000)

Issue 181577176