Skip to content

FIX(client) Address crash with pipewire and rtkit (Issue #6780) #7098

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.

Sign up for GitHub

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

Draft
Granahir2 wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −1
Draft

FIX(client) Address crash with pipewire and rtkit (Issue #6780) #7098

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/mumble/AudioInput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,8 @@ AudioInput::AudioInput()
connect(this, SIGNAL(doDeaf()), Global::get().mw->qaAudioDeaf, SLOT(trigger()), Qt::QueuedConnection);
connect(this, SIGNAL(doMute()), Global::get().mw->qaAudioMute, SLOT(trigger()), Qt::QueuedConnection);
connect(this, SIGNAL(doMuteCue()), Global::get().mw, SLOT(on_muteCuePopup_triggered()));

resetAudioProcessor();
Copy link
Copy Markdown
Member

@Hartmnt Hartmnt Mar 10, 2026

Choose a reason for hiding this comment

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

While this might fix the rtkit crash, I don't think this fix is correct for the following reason:

The resetAudioProcessor(); is called over and over again and usually terminates early because bResetProcessor is usually false except for the first run and when the child class calls initializeAudioMixer which can happen multiple times in the lifetime of an AudioInput.

Moving resetAudioProcessor to the constructor makes setting bResetProcessor to true a no-op which will very likely regress most if not all audio backends.

This needs more consideration and possibly a refactor.

Copy link
Copy Markdown
Member

@davidebeatrici davidebeatrici Mar 13, 2026

Choose a reason for hiding this comment

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

You're absolutely right. I wanted to mention the function's purpose and why moving it was potentially going to be a problem, but then I decided to focus on why it wouldn't be the correct solution anyway.

A refactor is definitely needed, albeit we should do it after switching to libxound.

}

AudioInput::~AudioInput() {
Expand Down Expand Up @@ -546,12 +548,14 @@ void AudioInput::initializeMixer() {
static_cast< unsigned int >(iEchoChannels * ((eEchoFormat == SampleFloat) ? sizeof(float) : sizeof(short)));

bResetProcessor = true;
resetAudioProcessor();

Comment on lines 550 to 552
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== AudioInput::initializeMixer definition and call sites =="
rg -nP --type=cpp -C4 '\bAudioInput::initializeMixer\s*\(|\binitializeMixer\s*\('

echo
echo "== AudioInput::resetAudioProcessor definition and call sites =="
rg -nP --type=cpp -C4 '\bAudioInput::resetAudioProcessor\s*\(|\bresetAudioProcessor\s*\('

echo
echo "== Sites using qmSpeex lock =="
rg -nP --type=cpp -C4 '\bQMutexLocker\s+\w+\s*\(&qmSpeex\)'

Repository: mumble-voip/mumble

Length of output: 12414

🏁 Script executed:

# View resetAudioProcessor() full implementation
sed -n '750,850p' src/mumble/AudioInput.cpp

echo "---"

# View encodeAudioFrame() to see qmSpeex lock scope
rg -n 'void AudioInput::encodeAudioFrame' -A 200 src/mumble/AudioInput.cpp | head -250

Repository: mumble-voip/mumble

Length of output: 10738

🏁 Script executed:

# Check where bRunning is set and how it relates to initializeMixer/encodeAudioFrame
rg -n 'bRunning\s*=' src/mumble/AudioInput.cpp | head -30

echo "---"

# Check if there's any synchronization around initializeMixer calls
rg -n 'while.*bRunning' src/mumble/AudioInput.cpp

echo "---"

# Look for any thread management or synchronization in backend files
rg -n 'bRunning' src/mumble/PAAudio.cpp -B2 -A2 | head -40

Repository: mumble-voip/mumble

Length of output: 114

🏁 Script executed:

# Get context around those bRunning assignments
sed -n '285,305p' src/mumble/AudioInput.cpp

echo "---"

# Look at the constructor to understand initialization order
sed -n '240,310p' src/mumble/AudioInput.cpp

Repository: mumble-voip/mumble

Length of output: 2393

🏁 Script executed:

# Check if backends stop audio before calling initializeMixer
# Look at PAAudio as a representative example
rg -n 'initializeMixer|bRunning' src/mumble/PAAudio.cpp -B2 -A2 | head -60

echo "---"

# Check threading model - is initializeMixer called from backend thread?
rg -n 'class PAAudioInput|void run|initializeMixer' src/mumble/PAAudio.cpp -B1 -A3 | head -100

Repository: mumble-voip/mumble

Length of output: 662

🏁 Script executed:

# Check if there are any settings changes or reconfigurations that trigger initializeMixer while running
rg -n 'initializeMixer|preferredFormat|preferredChannels' src/mumble/AudioInput.h -B2 -A2

echo "---"

# Search for any calls to initializeMixer outside of initial setup
rg -n 'initializeMixer' src/mumble/ --type=cpp | grep -v 'AudioInput.cpp\|AudioOutput.cpp\|ASIOInput\|OSS.cpp\|PAAudio\|WASAPI\|PulseAudio\|PipeWire\|JackAudio\|ALSAAudio'

echo "---"

# Check if there's any mechanism to restart/reinitialize audio dynamically
rg -n 'on.*Settings.*Changed|on.*Preferences|restartAudio' src/mumble/AudioInput.cpp -B1 -A3 | head -50

Repository: mumble-voip/mumble

Length of output: 458

🏁 Script executed:

# Look for the run() method in AudioInput to see when encodeAudioFrame is called
rg -n 'void AudioInput::run' -A 100 src/mumble/AudioInput.cpp | head -150

echo "---"

# Check if any Qt signals/slots trigger initializeMixer or settings changes
rg -n 'Q_SLOTS|public slots|private slots' src/mumble/AudioInput.h -A 5 | head -100

Repository: mumble-voip/mumble

Length of output: 146

🏁 Script executed:

# Search for run() method - it's inherited, so check the base class pattern
rg -n 'virtual void run|void.*run.*override' src/mumble/AudioInput.cpp -B2 -A5

echo "---"

# Search in all audio files for run() implementation
find src/mumble -name '*.cpp' -exec grep -l 'void.*::run()' {} \;

echo "---"

# Check the actual flow - look at PAAudioInput::run() specifically
rg -n 'void PAAudioInput::run|class PAAudioInput' -A 200 src/mumble/PAAudio.cpp | head -250

Repository: mumble-voip/mumble

Length of output: 444

🏁 Script executed:

# Get PAAudioInput::run() implementation
rg -n 'void PAAudioInput::run' -A 150 src/mumble/PAAudio.cpp

echo "---"

# Also check the callback mechanism - how is encodeAudioFrame called?
rg -n 'encodeAudioFrame|StreamCallback' src/mumble/PAAudio.cpp -B3 -A3

Repository: mumble-voip/mumble

Length of output: 387

🏁 Script executed:

# Find where encodeAudioFrame is called from
rg -n 'encodeAudioFrame|addMic' src/mumble/PAAudio.cpp -B2 -A2

echo "---"

# Check the main AudioInput::run() method that likely calls encodeAudioFrame
rg -n 'class AudioInput|AudioInput::run|void run' src/mumble/AudioInput.h

echo "---"

# Look for actual run() implementation in AudioInput.cpp
rg -n 'void.*run.*\{' src/mumble/AudioInput.cpp -A 100 | head -150

Repository: mumble-voip/mumble

Length of output: 358

🏁 Script executed:

# Find addMic implementation and how it relates to encodeAudioFrame
rg -n 'void.*addMic|void.*addEcho' src/mumble/AudioInput.cpp -B2 -A 20 | head -100

echo "---"

# Check the threading - is AudioInput::run() called as a QThread?
rg -n 'class AudioInput.*QThread|QThread.*start' src/mumble/AudioInput.h -B2 -A2

echo "---"

# Look for where audio actually starts - when run() begins
sed -n '125,200p' src/mumble/AudioInput.h

Repository: mumble-voip/mumble

Length of output: 5330

🏁 Script executed:

# Look at addMic continuation to see where encodeAudioFrame is called
sed -n '561,611p' src/mumble/AudioInput.cpp

echo "---"

# Check if there's any slot that could reinitialize mixer during runtime
rg -n 'onUserMuteDeafStateChanged|Q_SLOTS' src/mumble/AudioInput.cpp -A 10

Repository: mumble-voip/mumble

Length of output: 2222

Wrap resetAudioProcessor() call in qmSpeex lock to prevent data race with encodeAudioFrame().

resetAudioProcessor() mutates sesEcho and m_preprocessor without synchronization, while encodeAudioFrame() accesses these same objects under the qmSpeex lock. The asymmetric locking creates a potential UAF/data race if both execution paths overlap.

🔧 Proposed fix 
 void AudioInput::initializeMixer() {
 	bResetProcessor = true;
-	resetAudioProcessor();
+	{
+		QMutexLocker l(&qmSpeex);
+		resetAudioProcessor();
+	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bResetProcessor = true;
resetAudioProcessor();
bResetProcessor = true;
{
QMutexLocker l(&qmSpeex);
resetAudioProcessor();
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/mumble/AudioInput.cpp` around lines 550 - 552, Call to
resetAudioProcessor() must be performed while holding the qmSpeex mutex to match
encodeAudioFrame()'s locking and avoid races on sesEcho and m_preprocessor;
surround the resetAudioProcessor() invocation (and the bResetProcessor write if
not already protected) with the same qmSpeex lock used in encodeAudioFrame(),
ensuring sesEcho and m_preprocessor are modified only under that lock and
release it after the reset completes.

qWarning("AudioInput: Initialized mixer for %d channel %d hz mic and %d channel %d hz echo", iMicChannels, iMicFreq,
iEchoChannels, iEchoFreq);
if (uiMicChannelMask != 0xffffffffffffffffULL) {
qWarning("AudioInput: using mic channel mask 0x%llx", static_cast< unsigned long long >(uiMicChannelMask));
}

}

void AudioInput::addMic(const void *data, unsigned int nsamp) {
Expand Down Expand Up @@ -891,7 +895,6 @@ void AudioInput::encodeAudioFrame(AudioChunk chunk) {
}

QMutexLocker l(&qmSpeex);
resetAudioProcessor();

const std::int32_t gainValue = m_preprocessor.getAGCGain();

Expand Down
Loading