TapGestureDetector mutex is not locked error on Touchscreen #3655
Assignees
Labels
Comments
|
we are seeing this as well. |
igordmn
added a commit
to JetBrains/compose-multiplatform-core
that referenced
this issue
Sep 18, 2023
Fixes JetBrains/compose-multiplatform#3655 The issue is introduced after migration to `awaitPointerEventScope`, this code is indeed has a race condition, as it was assumed in the TODO. When we send Press/Release without any moves - we go via this path: ``` // awaitPointerEventScope isn't called until it receives the first event awaitPointerEventScope { // in the beginning of the method we have 2 events already in the queue launch { pressScope.reset() } // launch async, reset isn't called val down = awaitPress // don't suspend here, because we already have Press awaitReleaseOrCancelled() // don't suspend here, because we already have Release pressScope.release() // crash, because `pressScope.reset()` isn't yet called } ``` To avoid races, we should always access `pressScope` in `launch` (this way calls maintain order). The same approach is used in the original `TapGestureDetector.kt` (the change in commit 32de9dd) Also, the rest of the code doesn't have races with `pressScope` calls. ## Testing - haven't reproduced with mouse - reproduced with a simple test (added to OnClickTest)
igordmn
added a commit
to JetBrains/compose-multiplatform-core
that referenced
this issue
Sep 19, 2023
Fixes JetBrains/compose-multiplatform#3655 The issue is introduced after migration to `awaitPointerEventScope`, this code is indeed has a race condition, as it was assumed in the TODO. When we send Press/Release without any moves - we go via this path: ``` // awaitPointerEventScope isn't called until it receives the first event awaitPointerEventScope { // in the beginning of the method we have 2 events already in the queue launch { pressScope.reset() } // launch async, reset isn't called val down = awaitPress // don't suspend here, because we already have Press awaitReleaseOrCancelled() // don't suspend here, because we already have Release pressScope.release() // crash, because `pressScope.reset()` isn't called yet } ``` To avoid races, we should always access `pressScope` in `launch` (this way the calls maintain order). The same approach is used in the original `TapGestureDetector.kt` (the change in commit 32de9dd) Also, the rest of the code doesn't have races with `pressScope` calls. ## Testing - haven't reproduced with mouse - reproduced with a simple test (added to OnClickTest)
elijah-semyonov
pushed a commit
to JetBrains/compose-multiplatform-core
that referenced
this issue
Sep 21, 2023
Fixes JetBrains/compose-multiplatform#3655 The issue is introduced after migration to `awaitPointerEventScope`, this code is indeed has a race condition, as it was assumed in the TODO. When we send Press/Release without any moves - we go via this path: ``` // awaitPointerEventScope isn't called until it receives the first event awaitPointerEventScope { // in the beginning of the method we have 2 events already in the queue launch { pressScope.reset() } // launch async, reset isn't called val down = awaitPress // don't suspend here, because we already have Press awaitReleaseOrCancelled() // don't suspend here, because we already have Release pressScope.release() // crash, because `pressScope.reset()` isn't called yet } ``` To avoid races, we should always access `pressScope` in `launch` (this way the calls maintain order). The same approach is used in the original `TapGestureDetector.kt` (the change in commit 32de9dd) Also, the rest of the code doesn't have races with `pressScope` calls. ## Testing - haven't reproduced with mouse - reproduced with a simple test (added to OnClickTest)
|
@igordmn when are we going to get a release with this? We have customers complaining about this bug. |
|
1.5.10 (bugfixes + features for multiplatform) will be approximately in a month. Will it work for you? |
|
@igordmn That's an awfully long time away. If the release is a month away, it will have been almost 8 weeks between releases. Is it unreasonable to expect faster bug fix releases for regressions? |
|
We'll build 1.5.2 soon then |
|
Oh wow, I wasn't expecting that. That would be amazing! |
Walingar
pushed a commit
to JetBrains/compose-multiplatform-core
that referenced
this issue
Sep 25, 2023
Fixes JetBrains/compose-multiplatform#3655 The issue is introduced after migration to `awaitPointerEventScope`, this code is indeed has a race condition, as it was assumed in the TODO. When we send Press/Release without any moves - we go via this path: ``` // awaitPointerEventScope isn't called until it receives the first event awaitPointerEventScope { // in the beginning of the method we have 2 events already in the queue launch { pressScope.reset() } // launch async, reset isn't called val down = awaitPress // don't suspend here, because we already have Press awaitReleaseOrCancelled() // don't suspend here, because we already have Release pressScope.release() // crash, because `pressScope.reset()` isn't called yet } ``` To avoid races, we should always access `pressScope` in `launch` (this way the calls maintain order). The same approach is used in the original `TapGestureDetector.kt` (the change in commit 32de9dd) Also, the rest of the code doesn't have races with `pressScope` calls. ## Testing - haven't reproduced with mouse - reproduced with a simple test (added to OnClickTest)
|
@igordmn |
|
Just saw this on my app which is using compose |
|
@brendanw As I understand in your logs is that you founded this crash on Android device? |
|
I just had this error with 1.6.0 for one of my users (reported on Sentry.io). Affected platforms
Versions
To Reproduce
Stacktrace below: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
If a element has the onClick modifier on it and is touched via the Touchscreen and it is the first interaction with any of the buttons of the application, it crashes with exception: mutex is not locked.
Affected platforms
Select one of the platforms below:
Versions
To Reproduce
Steps and/or the code snippet to reproduce the behavior:
Box(modifier = Modifier.onClick({}).fillMaxSize()){}Expected behavior
Nothing should happen, as the onClick Funktion is empty.
Screenshots
Log
Additional context
Seems related to: #Kotlin/kotlinx.coroutines#1937
Im guessing the initial state of the mutex can't be correctly assumed by the Touch part specificly, since one Mouseclick on some other Button fixes all future interactions.
The text was updated successfully, but these errors were encountered: