Fix crash in UploadStarter on Samsung devices with Android 5#10877
Closed
malinajirka wants to merge 329 commits intorelease/13.7from
Closed
Fix crash in UploadStarter on Samsung devices with Android 5#10877malinajirka wants to merge 329 commits intorelease/13.7from
malinajirka wants to merge 329 commits intorelease/13.7from
Conversation
…g_fragment Add non-null check for activity in Gutenberg fragment
…g-non-null Reader PhotoViewer no longer crashes with null imgUrl
…lement-list-empty Fixing NoSuchElement list empty in PostDayViewsUseCase
Add task to setup dev dependencies
Create a Sequencing snackbar feature
|
You can test the changes on this Pull Request by downloading the APK here. |
shiki
approved these changes
Nov 29, 2019
WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt
Show resolved
Hide resolved
2 tasks
Generated by 🚫 dangerJS |
Contributor
|
This PR and the referenced issue can be closed right @shiki? |
Contributor
|
Yes! Thank you, @jd-alexander. Closing this in favor of #10878 which will be released with |
ParaskP7
added a commit
that referenced
this pull request
Sep 27, 2022
Warning Messages: "@synchronized annotation is not applicable to suspend functions and lambdas" It turns out that this '@synchronized' solution were already replacing the previous (d5b9948) 'Mutex' solution, and this, due to the fact that the existing (back then) 'Mutex' solution were causing crashes on Samsung devices running Android 5 (API Level 21). However, this repo, and thus both, the WordPress and Jetpack apps, are no longer supporting these devices as the current 'minSdkVersion' is '24' (Android 7). Thus, this '@synchronized' annotation can be now safely removed and the 'Mutex' solution reverting back. For more info see: - Fix crash in UploadStarter on Samsung devices with Android 5 #10877 (Introduced In): https://github.com/wordpress-mobile/WordPress-Android/ pull/10877/ - ExceptionInInitializerError #10827 (Crash Report): #10827 - NoSuchFieldException with AtomicReferenceFieldUpdater on Samsung Android 5.0.x devices #490 (Coroutines Issue): Kotlin/kotlinx.coroutines#490
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10827
Fixes a crash on Samsung devices running Android 5. The app is crashing because it can't find
_statevariable in the runtime. However, we don't directly use this variable in our code. As it turned out the variable is used inMutex.kt. This PR simply removes the usage ofMutexfrom our codebase and replaces it with@Synchronized.More info about why the app is crashing can be find in @jd-alexander comment.
To test:
I don't have a Samsung phone running Android 5. But we should at least verify it still works as expected on other phones.
PR submission checklist:
I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.Thank you @jd-alexander for the initial investigation which really helped me fix this issue!
@jkmassel We might want to consider merging this change as hotfix - wdyt? It crashed 3.6k times to 1.2k users in the last 90 days. I'm a bit worried that it might not work on Samsung devices running Android 5 at all - I haven't checked the stats though. I believe the change is very safe.