Skip to content

Commit a89e5ee

Browse files
Carmelo Piccioneclaude
andcommitted
Fix heap corruption and crashes from thread safety issues
Add mutex synchronization to prevent concurrent access to projectM state: - Add std::recursive_mutex to ProjectM class protecting RenderFrame, LoadPresetFile, LoadPresetData, SetWindowSize, SetPresetLocked, SetTexturePaths, ResetTextures, and BurnInTexture - Add std::mutex to PCM class protecting audio buffer writes (Add) and reads (UpdateFrameAudioData) from concurrent audio/render threads - Replace m_start from std::atomic<size_t> to plain size_t (now mutex-protected) - Implement EvalLibMutex lock/unlock with actual std::mutex (was no-op stubs) - Fix null pointer dereference in SpriteManager::Draw when m_transitioningPreset is nullptr after transition completes - Expose RenderMutex() accessor for external synchronization Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent db22c93 commit a89e5ee

6 files changed

Lines changed: 63 additions & 13 deletions

File tree

src/libprojectM/Audio/PCM.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "Audio/PCM.hpp"
22

3+
#include <mutex>
4+
35
namespace libprojectM {
46
namespace Audio {
57

@@ -17,6 +19,7 @@ void PCM::AddToBuffer(
1719
return;
1820
}
1921

22+
std::lock_guard<std::mutex> lock(m_pcmMutex);
2023
for (size_t i = 0; i < sampleCount; i++)
2124
{
2225
size_t const bufferOffset = (m_start + i) % AudioBufferSamples;
@@ -48,9 +51,12 @@ void PCM::Add(int16_t const* const samples, uint32_t channels, size_t const coun
4851

4952
void PCM::UpdateFrameAudioData(double secondsSinceLastFrame, uint32_t frame)
5053
{
51-
// 1. Copy audio data from input buffer
52-
CopyNewWaveformData(m_inputBufferL, m_waveformL);
53-
CopyNewWaveformData(m_inputBufferR, m_waveformR);
54+
// 1. Copy audio data from input buffer (lock to prevent tearing with audio thread writes)
55+
{
56+
std::lock_guard<std::mutex> lock(m_pcmMutex);
57+
CopyNewWaveformData(m_inputBufferL, m_waveformL);
58+
CopyNewWaveformData(m_inputBufferR, m_waveformR);
59+
}
5460

5561
// 2. Update spectrum analyzer data for both channels
5662
UpdateSpectrum(m_waveformL, m_spectrumL);
@@ -110,7 +116,7 @@ void PCM::UpdateSpectrum(const WaveformBuffer& waveformData, SpectrumBuffer& spe
110116

111117
void PCM::CopyNewWaveformData(const WaveformBuffer& source, WaveformBuffer& destination)
112118
{
113-
auto const bufferStartIndex = m_start.load();
119+
auto const bufferStartIndex = m_start;
114120

115121
for (size_t i = 0; i < AudioBufferSamples; i++)
116122
{

src/libprojectM/Audio/PCM.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <atomic>
1919
#include <cstdint>
2020
#include <cstdlib>
21+
#include <mutex>
2122

2223

2324
namespace libprojectM {
@@ -89,10 +90,12 @@ class PCM
8990
*/
9091
void CopyNewWaveformData(const WaveformBuffer& source, WaveformBuffer& destination);
9192

93+
std::mutex m_pcmMutex; //!< Protects the circular input buffer from concurrent access.
94+
9295
// External input buffer
9396
WaveformBuffer m_inputBufferL{0.f}; //!< Circular buffer for left-channel PCM data.
9497
WaveformBuffer m_inputBufferR{0.f}; //!< Circular buffer for right-channel PCM data.
95-
std::atomic<size_t> m_start{0}; //!< Circular buffer start index.
98+
size_t m_start{0}; //!< Circular buffer start index.
9699

97100
// Frame waveform data
98101
WaveformBuffer m_waveformL{0.f}; //!< Left-channel waveform data, aligned. Only the first WaveformSamples number of samples are valid.
Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
11
#include <projectm-eval.h>
22

3-
void projectm_eval_memory_host_lock_mutex() {}
4-
void projectm_eval_memory_host_unlock_mutex() {}
3+
#include <mutex>
4+
5+
static std::mutex s_evalMutex;
6+
7+
void projectm_eval_memory_host_lock_mutex()
8+
{
9+
s_evalMutex.lock();
10+
}
11+
12+
void projectm_eval_memory_host_unlock_mutex()
13+
{
14+
s_evalMutex.unlock();
15+
}

src/libprojectM/ProjectM.cpp

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828

2929
#include <Audio/PCM.hpp>
3030

31+
#include <mutex>
32+
3133
#include <Renderer/CopyTexture.hpp>
3234
#include <Renderer/PresetTransition.hpp>
3335
#include <Renderer/ShaderCache.hpp>
@@ -59,6 +61,7 @@ void ProjectM::PresetSwitchFailedEvent(const std::string&, const std::string&) c
5961

6062
void ProjectM::LoadPresetFile(const std::string& presetFilename, bool smoothTransition)
6163
{
64+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
6265
try
6366
{
6467
m_textureManager->PurgeTextures();
@@ -73,6 +76,7 @@ void ProjectM::LoadPresetFile(const std::string& presetFilename, bool smoothTran
7376

7477
void ProjectM::LoadPresetData(std::istream& presetData, bool smoothTransition)
7578
{
79+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
7680
try
7781
{
7882
m_textureManager->PurgeTextures();
@@ -87,17 +91,21 @@ void ProjectM::LoadPresetData(std::istream& presetData, bool smoothTransition)
8791

8892
void ProjectM::SetTexturePaths(std::vector<std::string> texturePaths)
8993
{
94+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
9095
m_textureSearchPaths = std::move(texturePaths);
9196
m_textureManager = std::make_unique<Renderer::TextureManager>(m_textureSearchPaths);
9297
}
9398

9499
void ProjectM::ResetTextures()
95100
{
101+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
96102
m_textureManager = std::make_unique<Renderer::TextureManager>(m_textureSearchPaths);
97103
}
98104

99105
void ProjectM::RenderFrame(uint32_t targetFramebufferObject /*= 0*/)
100106
{
107+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
108+
101109
// Don't render if window area is zero.
102110
if (m_windowWidth == 0 || m_windowHeight == 0)
103111
{
@@ -183,8 +191,15 @@ void ProjectM::RenderFrame(uint32_t targetFramebufferObject /*= 0*/)
183191
m_textureCopier->Draw(*renderContext.shaderCache, m_activePreset->OutputTexture(), false, false);
184192
}
185193

186-
// Draw user sprites
187-
m_spriteManager->Draw(audioData, renderContext, targetFramebufferObject, {m_activePreset, m_transitioningPreset});
194+
// Draw user sprites - only pass non-null presets to avoid dereferencing nullptr
195+
if (m_transitioningPreset)
196+
{
197+
m_spriteManager->Draw(audioData, renderContext, targetFramebufferObject, {m_activePreset, m_transitioningPreset});
198+
}
199+
else
200+
{
201+
m_spriteManager->Draw(audioData, renderContext, targetFramebufferObject, {m_activePreset});
202+
}
188203

189204
m_frameCount++;
190205
m_previousFrameVolume = audioData.vol;
@@ -251,7 +266,7 @@ void ProjectM::LoadIdlePreset()
251266

252267
void ProjectM::SetWindowSize(uint32_t width, uint32_t height)
253268
{
254-
/** Stash the new dimensions */
269+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
255270
m_windowWidth = width;
256271
m_windowHeight = height;
257272
}
@@ -339,6 +354,7 @@ auto ProjectM::UserSpriteIdentifiers() const -> std::vector<uint32_t>
339354

340355
void ProjectM::BurnInTexture(uint32_t openGlTextureId, int left, int top, int width, int height)
341356
{
357+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
342358
if (m_activePreset)
343359
{
344360
m_activePreset->BindFramebuffer();
@@ -356,8 +372,7 @@ void ProjectM::BurnInTexture(uint32_t openGlTextureId, int left, int top, int wi
356372

357373
void ProjectM::SetPresetLocked(bool locked)
358374
{
359-
// ToDo: Add a preset switch timer separate from the display timer and reset to 0 when
360-
// disabling the preset switch lock.
375+
std::lock_guard<std::recursive_mutex> lock(m_renderMutex);
361376
m_presetLocked = locked;
362377
m_presetChangeNotified = locked;
363378
}
@@ -514,6 +529,11 @@ auto ProjectM::PCM() -> libprojectM::Audio::PCM&
514529
return m_audioStorage;
515530
}
516531

532+
auto ProjectM::RenderMutex() -> std::recursive_mutex&
533+
{
534+
return m_renderMutex;
535+
}
536+
517537
void ProjectM::Touch(float, float, int, int)
518538
{
519539
// UNIMPLEMENTED

src/libprojectM/ProjectM.hpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <Audio/PCM.hpp>
2828

2929
#include <memory>
30+
#include <mutex>
3031
#include <string>
3132
#include <vector>
3233

@@ -196,6 +197,14 @@ class PROJECTM_EXPORT ProjectM
196197

197198
auto PCM() -> Audio::PCM&;
198199

200+
/**
201+
* @brief Returns a reference to the render mutex for external synchronization.
202+
*
203+
* This mutex must be held when calling any method that modifies or reads projectM state
204+
* from a thread other than the render thread (e.g., loading presets, adding PCM data).
205+
*/
206+
auto RenderMutex() -> std::recursive_mutex&;
207+
199208
auto WindowWidth() -> int;
200209

201210
auto WindowHeight() -> int;
@@ -291,6 +300,8 @@ class PROJECTM_EXPORT ProjectM
291300
bool m_presetLocked{false}; //!< If true, the preset change event will not be sent.
292301
bool m_presetChangeNotified{false}; //!< Stores whether the user has been notified that projectM wants to switch the preset.
293302

303+
std::recursive_mutex m_renderMutex; //!< Protects all projectM state from concurrent access across threads.
304+
294305
std::unique_ptr<PresetFactoryManager> m_presetFactoryManager; //!< Provides access to all available preset factories.
295306

296307
Audio::PCM m_audioStorage; //!< Audio data buffer and analyzer instance.

src/libprojectM/ProjectMCWrapper.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,6 @@ template<class BufferType>
393393
static auto PcmAdd(projectm_handle instance, const BufferType* samples, unsigned int count, projectm_channels channels) -> void
394394
{
395395
auto* projectMInstance = handle_to_instance(instance);
396-
397396
projectMInstance->PCM().Add(samples, channels, count);
398397
}
399398

0 commit comments

Comments
 (0)