Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-61725

QSoundEffect (pulseaudio variant) has locking issues

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.9.1
    • Fix Version/s: 5.9.2
    • Component/s: Multimedia
    • Labels:
      None
    • Commits:
      53538ff9492c5889db3bcbbbe17bc270d1e83cb1

      Description

      I'm creating this issue as advised by Christian to track the fixing of problems related to QSoundEffect's pulseaudio implementation. The discussion started initially on gerrit.

      A little backstory copied from there:

      We've been using qtmultimedia 5.8.0 without commit 639284a1beb2 ("PulseAudio: make sound effect implementation more robust") and ran into a race condition where it was possible to play a sound effect before the pulseaudio connection was established resulting in a segfault.

      We backported this patch and it fixed the crashes, but in our internal code review it was pointed out that this m_lockCount doesn't make any sense - if anything it only hides an actual locking issue.

      While the previous crash no longer occurs, we now experience a different problem: if pulseaudio daemon can't be started for some reason and pa_context_connect() fails twice, the calling program aborts due to a NULL mutex. Obviously a problem in the error path - this only occurs in specific circumstances currently (libpulse and libpulse-mainloop-glib must be available, but not the pulseaudio executable). Looks like a double unlock, but I haven't investigated that very much yet as we do have the pulseaudio binary in our normal setup.

      However this will become a problem once we introduce a change that we require, as there's another problem we're currently facing. The QSoundEffect implementation tries to connect to a default pulseaudio server (calling pa_context_connect() without any server string). Internally pulseaudio spawns a new pulseaudio instance if it can't find any running unless PA_CONTEXT_NOAUTOSPAWN flag is passed to it. This second instance races with our primary pulseaudio daemon. The two programs are run as different users, so once the Qt instance creates its runtime directory, the primary process can't overwrite the files within.

      Our proposition for that is to introduce an environment variable QT_PULSE_SERVER_STRING which - if specified - would be passed to pa_context_connect(). This routine would then fail if it can't connect to this specified server. This is where the error path bug comes in - the calling program crashes if pa_context_connect() fails both the first attempt and the second reconnect.

      An alternative idea is to have a different variable e.g. QT_PULSE_NOAUTOSPAWN which would make QSoundEffect pass the PA_CONTEXT_NOAUTOSPAWN flag to pa_context_connect(). But even then, the runtime directory is created anyway, which looks like a braindead behavior of pulseaudio.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            brgl Bartosz Golaszewski
            Reporter:
            brgl Bartosz Golaszewski
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews

                There are no open Gerrit changes