Skip to content

fix(memory): Fix memory leak for audio events when pausing the game#2731

Open
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_memory_leak_audio_requests
Open

fix(memory): Fix memory leak for audio events when pausing the game#2731
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_memory_leak_audio_requests

Conversation

@Caball009
Copy link
Copy Markdown

Pausing the game leaks audio events that were in the audio request container at the time. This PR fixes that.

This code is called to get rid of some of the audio requests when pausing the game:

//Get rid of PLAY audio requests when pausing audio.
std::list<AudioRequest*>::iterator ait;
for (ait = m_audioRequests.begin(); ait != m_audioRequests.end(); /* empty */)
{
AudioRequest *req = (*ait);
if( req && req->m_request == AR_Play )
{
deleteInstance(req);
ait = m_audioRequests.erase(ait);
}
else
{
ait++;
}
}

AudioRequest::m_pendingEvent can be an owning raw pointer, though, which the destructor of AudioRequest should delete when needed. It currently doesn't, which is why the leak happens.

There's one exception where the ownership of the audio event is taken away from the audio request:

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related Fix Is fixing something, but is not user facing labels May 19, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 19, 2026

Greptile Summary

This PR fixes a memory leak in audio handling when the game is paused: AR_Play requests that are dropped from the queue before processing never had their m_pendingEvent pointer freed, because AudioRequest::~AudioRequest() had an empty body. The fix adds a conditional delete m_pendingEvent in the destructor guarded by m_usePendingEvent, and converts playAudioEvent from void to Bool so processRequest knows whether to nullify the pointer (preventing a double-free on the normal processing path).

  • AudioRequest.cpp: Destructor now deletes m_pendingEvent when m_usePendingEvent is true, correctly freeing events in the pause-discard path and the skipped-checkForSample path in processRequestList.
  • MilesAudioManager.cpp / .h: playAudioEvent returns FALSE only in the early-exit (no AudioEventInfo) case where ownership wasn't transferred; it returns TRUE in all other paths where the event has been handed to a PlayingAudio object (which carries m_cleanupAudioEventRTS = true by default and is released through releasePlayingAudio on failure).
  • Ownership contract: releaseAudioEventRTS is simply delete + nullptr, so the raw delete m_pendingEvent in the destructor is semantically equivalent and consistent with existing deletion patterns across the codebase.

Confidence Score: 4/5

Safe to merge — targeted destructor fix with correct ownership accounting through all audio-event lifecycle paths.

The fix correctly threads ownership through three distinct paths (pause-discard, normal-play, no-info early exit) and the PlayingAudio default of m_cleanupAudioEventRTS = true ensures the event is always freed via releasePlayingAudio in playAudioEvent failure branches.

MilesAudioManager.cpp — the ownership contract between playAudioEvent's return value and the m_cleanupAudioEventRTS default on PlayingAudio.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/Audio/AudioRequest.cpp Adds destructor body that deletes m_pendingEvent when m_usePendingEvent is true — the core fix for the leak.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp playAudioEvent now returns Bool tracking ownership; processRequest nulls m_pendingEvent on TRUE return to prevent double-free.
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h Declaration updated from void to Bool for playAudioEvent — minimal, correct change.

Reviews (1): Last reviewed commit: "fix(memory): Fix memory leak for audio e..." | Re-trigger Greptile

{
// TheSuperHackers @info The audio event is no longer owned by this request, because it was just processed.
// Reset pointer to prevent the destructor of the request from calling delete on the audio event.
req->m_pendingEvent = nullptr;
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I could add req->m_usePendingEvent = FALSE here if desired. I don't think it makes a functional difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Gen Relates to Generals Memory Is memory related Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant