jMonkeyEngine / jMonkeyEngine/jmonkeyengine

Assertion failures in audio when deleting loaded objects, incorrect native disposal

Open
#1,864 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
4.3k
Forks
1.2k
Avg merge
4d 7h
Merged PRs (30d)
14

Description

Please consider the following small application.

In frame 0 it plays an AudioBuffer and AudioStream, then later in frame 10 it stops playing both and then destroys the audio.

In a real-world scenario this might be because those audios will not be played again in the lifetime of the app, and it might be worth freeing that space up.

public class Main extends SimpleApplication {

  private AudioBuffer audioBuffer;
  private AudioStream audioStream;
  private AudioNode bufferNode;
  private AudioNode streamNode;
  private int frame = 0;

  public static void main(String[] args) {
    Main app = new Main();
    app.setSettings(new AppSettings(true));
    app.start();
  }

  @Override
  public void simpleInitApp() {
    AudioKey bufferKey = new AudioKey("example.wav", false, false);
    audioBuffer = (AudioBuffer) assetManager.loadAudio(bufferKey);
    bufferNode = new AudioNode(audioBuffer, bufferKey);

    AudioKey streamKey = new AudioKey("example.wav", true, true);
    audioStream = (AudioStream) assetManager.loadAudio(streamKey);
    streamNode = new AudioNode(audioStream, streamKey);
  }

  @Override
  public void simpleUpdate(float tpf) {
    if (frame == 0) {
      bufferNode.play();
      streamNode.play();
    } else if (frame == 10) {
      bufferNode.stop();
      streamNode.stop();
      // Note, these are equivalent to audioRenderer.deleteAudioData(audio)
      audioBuffer.deleteObject(audioRenderer);
      audioStream.deleteObject(audioRenderer);
    }
    frame++;
  }

}

Issue 1:
When run with assertions enabled, this quickly crashes:

Exception in thread "jME3 Audio Decoder" java.lang.AssertionError: 5 != 1
	at com.jme3.audio.lwjgl.LwjglALC.alcGetInteger(LwjglALC.java:98)
	at com.jme3.audio.openal.ALAudioRenderer.isDisconnected(ALAudioRenderer.java:906)
	at com.jme3.audio.openal.ALAudioRenderer.checkDevice(ALAudioRenderer.java:895)
	at com.jme3.audio.openal.ALAudioRenderer.run(ALAudioRenderer.java:286)
	at java.base/java.lang.Thread.run(Thread.java:832)

The reason being, LwjglALC.alcGetInteger expects the passed IntBuffer to be correctly set up before passing in. Most times it is, because typical usage of ib in ALAudioRenderer coincidentally leaves it in pos 0 with limit 1 which is what's expected. The AudioStream deletion in ALAudioRenderer.deleteAudioData however is an atypical case due to handling of multiple ids.
This issue is easily fixed by adding ib.position(0).limit(1); to ALAudioRenderer.isDisconnected() before the call.


Issue 2:
Once the above is fixed the app runs correctly, but when pressing Esc to exit, it crashes again:

java.lang.AssertionError
	at com.jme3.util.NativeObjectManager.deleteNativeObject(NativeObjectManager.java:132)
	at com.jme3.util.NativeObjectManager.deleteAllObjects(NativeObjectManager.java:214)
	at com.jme3.audio.openal.ALAudioRenderer.destroyOpenAL(ALAudioRenderer.java:239)
	at com.jme3.audio.openal.ALAudioRenderer.cleanup(ALAudioRenderer.java:325)
	at com.jme3.app.LegacyApplication.destroy(LegacyApplication.java:816)
	at com.jme3.system.lwjgl.LwjglWindow.deinitInThread(LwjglWindow.java:644)
	at com.jme3.system.lwjgl.LwjglWindow.run(LwjglWindow.java:680)
	at java.base/java.lang.Thread.run(Thread.java:832)

The reason being, that ALAudioRenderer registers AudioBuffers with it's NativeObjectManager, but never clears this registration. The NativeObjectManager will retain references to the audio in a bad state forever until the graceful app shutdown (or any other reason for the context to be destroyed/reset) starts to clear house, and trips over it.
Fixing this will be a bit tricker, and something best left to the platform experts - My intuition told me the correct solution was be to make NativeObjectManager.enqueueUnusedObject public, and call it inside ALAudioRenderer.deleteAudioData so that it effectively de-registers the audio again.
The assertion still pops because ALAudioRenderer.deleteAudioData calls resetObject before the NativeObjectManager can get around to deleting it. We can remove this call though, because NativeObjectManager calls this itself.
Unfortunately when NativeObjectManager runs it's delete it also calls obj.deleteObject(rendererObject);, when means I have created an infinite loop back around to ALAudioRenderer.deleteAudioData. It just crashes on loop 2 because the object is already deleted.
So, there is some kind of structural power struggle here between AudioRenderer which provides the obvious way for a dev to delete audio, and the NativeObjectManager which is private but set up to be the controller of the data with calls to the AudioRenderer deletion method as part of it's process.
In the end I backed all that out and my hack solution was to add this to ALAudioRenderer - which works well enough but is obviously untidy.

public void deleteAudioDataProperly(AudioData data) {
    if (data instanceof AudioStream) {
      // AudioStreams don't register with objManager so we can simply delete it directly
      deleteAudioData(data);
    } else if (data instanceof AudioBuffer) {
      // This will indirectly call deleteAudioData(data) when objManager deletes objects in it's queue
      objManager.enqueueUnusedObject(data);
    } else {
      throw new UnsupportedOperationException(data.getClass() + " - " + data);
    }
  }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the two assertion failures from the Java example, then trace ALAudioRenderer.isDisconnected(), deleteAudioData(), destroyOpenAL(), and NativeObjectManager.deleteNativeObject(). Check how AudioBuffer and AudioStream registrations differ. Done means deleting both loaded objects and exiting cleanly with assertions enabled, without recursive or duplicate native-object deletion.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
audio-video-rtc
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.