Skip to content

Replace shared StringBuilders to avoid threading conflicts

Robert Stone requested to merge rhstone/vlc-android:string-builder into master

Description

I observed an exception thrown from two co-routines sharing the same StringBuilder instance. After seeing this exception, I searched through the code for the other locations where a StringBuilder is being shared. I replaced all occurrences of the non thread-safe pattern with either string concatenation or Kotlin templates.

Motivation and Context

Observed stack trace and failure to display notification due to a race condition between two threads:

E/VLC/PlaybackService: Failed to display notification
    java.lang.StringIndexOutOfBoundsException: length=70; regionStart=0; regionLength=78
        at java.lang.StringFactory.newStringFromChars(StringFactory.java:258)
        at java.lang.StringBuilder.toString(StringBuilder.java:413)
        at org.videolan.vlc.gui.helpers.NotificationHelper.createPlaybackNotification(NotificationHelper.kt:71)
        at org.videolan.vlc.PlaybackService$showNotificationInternal$1.invokeSuspend(PlaybackService.kt:743)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
E/VLC/PlaybackService: Failed to display notification
    java.lang.StringIndexOutOfBoundsException: length=70; regionStart=0; regionLength=78
        at java.lang.StringFactory.newStringFromChars(StringFactory.java:258)
        at java.lang.StringBuilder.toString(StringBuilder.java:413)
        at org.videolan.vlc.gui.helpers.NotificationHelper.createPlaybackNotification(NotificationHelper.kt:71)
        at org.videolan.vlc.PlaybackService$showNotificationInternal$1.invokeSuspend(PlaybackService.kt:743)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

How Has This Been Tested?

Notifications were observed to be identical post change.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING section of the README document.

Related: !827 (merged)

Merge request reports