Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix StackOverflowError while recording voice message [PSF-1065] #6222

Merged
merged 3 commits into from
Jun 6, 2022

Conversation

onurays
Copy link
Contributor

@onurays onurays commented Jun 1, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Start recording a voice message and lock the mic.
  • Wait for 2 minutes (max length of a voice message)
  • The app crashes

Motivation and context

Fixes #6137

Screenshots / GIFs

Tests

  • Start recording a voice message and wait for 2 minutes
  • Send the record, delete the record and everything should work as expected

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@onurays onurays changed the title Fix StackOverflowError while recording voice message Fix StackOverflowError while recording voice message [PSF-1065] Jun 1, 2022
@onurays onurays requested review from a team, Florian14 and fedrunov and removed request for a team June 1, 2022 14:04
@github-actions
Copy link

github-actions bot commented Jun 1, 2022

Unit Test Results

146 files  ±0  146 suites  ±0   2m 9s ⏱️ -36s
236 tests ±0  236 ✔️ ±0  0 💤 ±0  0 ±0 
788 runs  ±0  788 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 0b9db0e. ± Comparison against base commit 033b877.

♻️ This comment has been updated with latest results.

visibleBarHeights = visibleBarHeights.subList(visibleBarHeights.size - maxVisibleBarCount, visibleBarHeights.size)
visibleBarHeights = mutableListOf<FFT>().apply {
addAll(visibleBarHeights.subList(visibleBarHeights.size - maxVisibleBarCount, visibleBarHeights.size))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we want to keep the last maxVisibleBarCount items on the list.

Can it be rewritten like this:

if (visibleBarHeights.size > maxVisibleBarCount) {
        visibleBarHeights = visibleBarHeights.takeLast(maxVisibleBarCount).toMutableList()
        // Or, but maybe less clear
        visibleBarHeights = visibleBarHeights.drop(maxVisibleBarCount - visibleBarHeights.size).toMutableList()
}

Alternatively we could drop the first items like this:

            while (visibleBarHeights.size > maxVisibleBarCount) {
                visibleBarHeights.removeAt(0)
            }

which has the advantage of not creating a new list. Also it should in fact only drop one item, each time we add an item and make the list too long.

To me those both suggestions are easier to read.

Copy link
Contributor Author

@onurays onurays Jun 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good ideas, thanks!

@onurays onurays merged commit bae830d into develop Jun 6, 2022
@onurays onurays deleted the feature/ons/fix_voice_message_stackoverflow branch June 6, 2022 09:58
@github-actions
Copy link

github-actions bot commented Jun 6, 2022

Matrix SDK

Integration Tests Results:

  • [org.matrix.android.sdk.session]
    = passed=19 failures=1 errors=0 skipped=3
  • [org.matrix.android.sdk.account]
    = passed=3 failures=0 errors=0 skipped=2
  • [org.matrix.android.sdk.internal]
    = passed=27 failures=1 errors=0 skipped=1
  • [org.matrix.android.sdk.ordering]
    = passed=16 failures=0 errors=0 skipped=0
  • [org.matrix.android.sdk.PermalinkParserTest]
    = passed=2 failures=0 errors=0 skipped=0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long voice message: crash with StackOverflowError
4 participants