Improve overall stability: fix memory leaks, crashes, and code correctness#119
Open
shiena wants to merge 16 commits intostechyo:masterfrom
Open
Improve overall stability: fix memory leaks, crashes, and code correctness#119shiena wants to merge 16 commits intostechyo:masterfrom
shiena wants to merge 16 commits intostechyo:masterfrom
Conversation
Rename GDEXAMPLE_ header guard to STEAM_AUDIO_ to match project convention. Fix init_ext/uninit_ext declarations to match their definitions which take ModuleInitializationLevel parameter.
The same value was stored twice in succession with no intervening code.
Use Ref<AudioStreamPlayback> instead of Ref<AudioStream> to match the function's declared return type.
Prevents use of uninitialized godot_log_level if Steam Audio adds new log levels in the future.
static in a header creates separate copies per translation unit (ODR violation). inline ensures a single shared instance across all TUs.
- SteamAudioStream::parent = nullptr
- SteamAudioStreamPlayback::is_active{false}
- SteamAudioDynamicGeometry::is_init{false}
Matches the pattern used by SteamAudioGeometry which properly initializes
its atomic booleans in its constructor.
num_refl_threads and max_num_refl_srcs are int members but had float getters/setters, causing implicit truncation. Changed to int to match the member types and Variant::INT property bindings.
iplStaticMeshRelease takes a pointer and nullifies the handle. Using auto (copy) meant the vector's handles were not nullified. Changed to auto& so the release operates on the actual vector elements.
Previously these warnings were emitted every frame, flooding the console and impacting performance.
- Notify the condition variable after setting is_running=false so the reflection thread can wake up and exit its wait loop. - Check refl_thread.is_valid() before calling wait_to_finish() to avoid crashing when get_global_state() was never called. These fixes address the root cause of the crash that led to the destructor being commented out in register_types.cpp.
iplSceneCreate already returns the scene with refcount=1. The extra iplSceneRetain bumped it to 2, but only one iplSceneRelease was called in the destructor, leaking the scene.
Check return values of iplSourceCreate, iplDirectEffectCreate, iplReflectionEffectCreate, iplAudioBufferAllocate (7 calls), and iplStaticMeshCreate using the existing handleErr() utility.
Check that MeshInstance3D has a mesh and CollisionShape3D has a shape before attempting to create IPL static meshes. Prevents null dereference crashes when geometry nodes have no mesh/shape assigned.
Remove local state from server when Player exits the scene tree, and clear the listener pointer when Listener exits. This prevents dangling pointers in the server when nodes are removed from the tree, matching the pattern already used by SteamAudioGeometry.
If mix_audio returns more frames than the allocated IPL audio buffer size, cap the count to frameSize to avoid writing beyond buffer bounds.
The destructor was missing release calls for reflection effect, ambisonics decode effects (x2), and ambisonics encode effect. These were allocated in init_local_state() but never freed, leaking memory every time a SteamAudioPlayer was destroyed. Fixes stechyo#77
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stability and robustness improvements based on code review. Fixes memory leaks, crashes, deadlocks, and strengthens error handling and type safety.
Changes
Memory leaks / Resource management
iplSceneRetainthat prevented scene from being freedauto&in range-for loops soiplStaticMeshReleasenullifies actual vector elementsCrash / Deadlock fixes
is_running=falseso reflection thread can exitrefl_thread.is_valid()beforewait_to_finish()EXIT_TREEnotification in Player and Listener to prevent dangling pointersError handling
handleErr()checks foriplSourceCreate,iplDirectEffectCreate,iplReflectionEffectCreate,iplAudioBufferAllocate,iplStaticMeshCreateType safety / Code correctness
register_types.hppget_inner_stream_playback()null caselog_callbackswitch to handle future log levelsembree_devfromstatictoinlinein header to fix ODR violationparent,is_active,is_init)float/inttype mismatch in config getters/settersMinor improvements
is_local_state_init.store(false)in destructor