Skip to content
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.

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

instances leaked at exit with AudioStreamPlayer #76745

Open
eldidou opened this issue May 5, 2023 · 3 comments
Open

instances leaked at exit with AudioStreamPlayer #76745

eldidou opened this issue May 5, 2023 · 3 comments

Comments

@eldidou
Copy link
Contributor

eldidou commented May 5, 2023

Godot version

Godot_v4.0.2-stable_linux.x86_64

System information

Ubuntu 21.10

Issue description

At exit, if a sound is still playing errors are displayed about leaked resources.

WARNING: ObjectDB instances leaked at exit (run with --verbose for details).
     at: cleanup (core/object/object.cpp:1982)
Leaked instance: AudioStreamPlaybackOggVorbis:-9222506721203714948
Leaked instance: OggPacketSequence:-9222509469982784387 - Resource path: res://bip.ogg::OggPacketSequence_7k54j
Leaked instance: AudioStreamOggVorbis:-9222508920226970498 - Resource path: res://bip.ogg
Leaked instance: OggPacketSequencePlayback:-9222506171447901057
Hint: Leaked instances typically happen when nodes are removed from the scene tree (with `remove_child()`) but not freed (with `free()` or `queue_free()`).
ERROR: Resources still in use at exit (run with --verbose for details).
   at: clear (core/io/resource.cpp:489)
Resource still in use: res://bip.ogg::OggPacketSequence_7k54j (OggPacketSequence)
Resource still in use: res://bip.ogg (AudioStreamOggVorbis)

It doesn't always happen, it's maybe around 50% of the time.
It happens only when launched outside of the editor using command line.

Steps to reproduce

  • Create a scene with the root node with this script:
extends Node


func _ready() -> void:
    # play a sound
    var asp = AudioStreamPlayer.new()
    asp.stream = load('res://bip.ogg')
    self.add_child(asp)
    asp.play()

    # quit before end of sound
    await get_tree().create_timer(0.3).timeout
    get_tree().quit()
  • launch from command line:
Godot_v4.0.2-stable_linux.x86_64 -t main.tscn

Minimal reproduction project

leaking_resources.zip

@MJacred
Copy link
Contributor

MJacred commented May 10, 2023

Related to #34495

@bluenote10
Copy link
Contributor

I had a look into this issue and it looks like it is also caused by #51296.

I noticed that when calling audio_stream_player.stop() and letting a sufficient amount of time pass until shutting down the app, there are no leaks.

Interestingly, AudioStreamPlayer already contains a reasonably looking logic for NOTIFICATION_PREDELETE to do the necessary cleanup:

case NOTIFICATION_PREDELETE: {
for (Ref<AudioStreamPlayback> &playback : stream_playbacks) {
AudioServer::get_singleton()->stop_playback_stream(playback);
}
stream_playbacks.clear();

This is basically the same stop():

void AudioStreamPlayer::stop() {
for (Ref<AudioStreamPlayback> &playback : stream_playbacks) {
AudioServer::get_singleton()->stop_playback_stream(playback);
}
stream_playbacks.clear();

So why does it not work as expected? The issue is that #51296 has introduce a Ref<AudioStreamPlayback> on the audio server side that is handled asynchronously:

Ref<AudioStreamPlayback> stream_playback;

Internally AudioServer::get_singleton()->stop_playback_stream(...) doesn't delete these playback references synchronously, but only sets their state to FADE_OUT_TO_DELETION:

godot/servers/audio_server.cpp

Lines 1169 to 1186 in c12d635

void AudioServer::stop_playback_stream(Ref<AudioStreamPlayback> p_playback) {
ERR_FAIL_COND(p_playback.is_null());
AudioStreamPlaybackListNode *playback_node = _find_playback_list_node(p_playback);
if (!playback_node) {
return;
}
AudioStreamPlaybackListNode::PlaybackState new_state, old_state;
do {
old_state = playback_node->state.load();
if (old_state == AudioStreamPlaybackListNode::AWAITING_DELETION) {
break; // Don't fade out again.
}
new_state = AudioStreamPlaybackListNode::FADE_OUT_TO_DELETION;
} while (!playback_node->state.compare_exchange_strong(old_state, new_state));
}

This means that the audio server holds on to these references for "a bit longer" to possibly do a fade out, and only free them later. During a process shutdown this doesn't make sense, and these references from the audio server are then reported as leaks.

So it should be possible to fix this issue by adding some corresponding shutdown cleanup logic on the audio server side. However the more I understand the code, the more I feel that #51296 has made things in the audio server more complicated than necessary.

@GustJc
Copy link
Contributor

GustJc commented Jul 14, 2024

I can confirm this is still present on 4.2.2 on a windows build game.

A quick fix for this is to stop all audiostreams and await 0.1s before exiting.
Here a example on how you'd intercept the exit button signal to stop the audio.

func _notification(what):
	if what == NOTIFICATION_WM_CLOSE_REQUEST:
		get_tree().set_auto_accept_quit(false) # or change project settings config
		print("Exiting")
		%LevelAudio.stop()
		await get_tree().create_timer(0.1).timeout
		get_tree().quit() # default behavior

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

No branches or pull requests

5 participants