Remove events with sequencer playing

[vustav]

I'll start by saying I'm not sure if this is a bug or me doing something wrong.

When I delete events with the sequencer playing I get this crash:

The only way to avoid it is to keep a reference to the event. Sometimes I can get rid of it after a while but not always (and I don't know how to get the info on when it is safe), so to keep the app running safely I have to keep removed events forever which could get out of hand after a while.

The way I intuitively tried to do it was to enqueue the event for removal and then at some point check if the event is removed and if it is, release it for garbage collection. The only check I've seen is isSequenced(), and it never turns false. Since clearing the array of events works sometimes I assume they do get removed and at some point are safe to release but I just don't do it right (or there's a bug)?

The wiki mentions (in Instrument.removeEvent) that a method setDeletable(true) in AudioEvent is adviced if the sequencer is playing, but it seems it doesn't exist.

[vustav]

I see now I posted a similar issue a year ago. I hadn't figured out that keeping the events prevents crashes then, so some new info at least!

[igorski]

Hi @vustav

You need to keep a reference to the event as otherwise the garbage collection process (assuming you are writing in Java/Kotlin) will invoke the destructors of the event.

But I understood from your previous issue and the information provided here that you are removing the events with the sequencer playing. This should be a completely safe operation where you can break the references to the events after removing them from the sequencer.

audioEvent->removeFromSequencer();

this however can fail if the instrument that owns the event is destroyed before the removal of the events. If this happens in the same sweep, the instrument should be removed last. I looked at the steps you mentioned in #173 and the correct flow should be:

for(events){
    event.removeFromSequencer();
}
instrument.getAudioChannel().getProcessingChain().reset();

and note that you should not call the delete() methods of the classes (these are public as they need to be called by the finalizer process of the garbage collector, but are not part of the API, a sad side effect of the native bindings). You can break the references to the events and instruments and they will be cleaned up automatically when the GC decides to do its work. They are however immediately removed from the engine and should no longer affect audio output.

[vustav]

Thanks for answering!

These crashes occur when deleting steps in a sequencer, so instruments and effects aren't deleted. Number of steps are reduced tough if that makes a difference.

These are all the steps involving the engine:

• sequencerController.updateMeasures(1, steps);
• sequencerController.setTempo(tempo, BEAT_AMOUNT, BEAT_UNIT);
• event.removeFromSequencer() on all events that should be removed (and add them to an array to avoid crashes).
• event.setEventStart(posInSamples) on all remaining events

If I don't keep references to the events after calling removeFromSequencer() it crashes almost immediately. If I keep references and clear the array when the sequencer is stopped it crashes after a while when playing again. removeFromSequencer() is all that is called. Keeping the references forever is the only thing that doesn't crash.

[igorski]

Alright @vustav

I had a look at your issue and I'll be honest and say I'm a little bit baffled how this crashes after reading the steps you provided in your last post 🙈

I have updated the repo and example Activity to address and debug this issue. If you pull the latest changes and build both the library and example Activity, the newly added "switch pattern" button (at the bottom of the screen) should replicate your scenario. You can click this button while playing to remove all events (of the bass line) and replace them with new ones. You can click this button to oblivion and it shouldn't crash.

I have updated the API for both BaseInstrument and BaseAudioEvent classes though. Instead of calling delete() (which I wrote you shouldn't do, however the example Activity did), there is now a dispose() method which you should call when either an event or instrument will no longer be used in your application. Calling dispose() should be a safe operation (as made evident by the example Activity's createBassPattern() function.

I would like to ask you to give the renewed engine and example Activity a spin and see if the issue still occurs for you. If it doesn't (which I hope it does 😬 ) it could indicate that its maybe not a matter of cleanup, but rather of creation of your events). If it does reoccur, could you state your target environment details and how you are writing your code (e.g. Java/Kotlin, which JDK versions, etc.). I have to be honest that there is an issue with garbage collection/destructor onging with SWIG (the wrapper for native and Java/Kotlin code) which I raised with them, but I don't think that issue will lead to the crashes you described.

[vustav]

Thanks! The example app and its new button works great. My app still crashes even with dispose() so that leaves out a bug in the engine I guess!

Can I ask you as someone with better insight to the engine (and computers in general I suppose) if you have any guesses as to what it might be? To me it looks like I somehow create more events than intended and that they for that reasons don't get disposed of properly and leads to this crash when the objects holding them are garbage collected. Or something along those lines. Would java have the power to get rid of things that are still used by something else, in this case the engine? That it doesn't crash as long as I keep references is the only lead I have.

I use java and JDK is 17.0.7

[vustav]

I replaced your createBassPattern() with a new method createDrumPattern() that's exactly the same but disposes all events in _drumEvents and recreates the drums instead. Mashing that one leads to:

Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x1010300 in tid 4875 (AudioTrack), pid 4851 (wengine.example)

... unfortunatelly I can't get a better stack trace than that but it looks like it|s the same one I get. I use both SynthEvents and SampleEvents when I get this crash.

[igorski]

Hi @vustav that is... odd.

I have created this branch to also replace the drum events, similar to the synth events, but I get no crashes no matter how hard I keep smashing that button... use OpenSL or AAudio driver or anything else... Does the crash reproduce for you in that branch ? Can you confirm whether your implementation for drum event replacement is the same as mine (you can view the diff here for easy reference) ?

If it does reproduce, could you confirm whether you are also building the example using Java 17.0.7 and what the specs are of the device / emulator you are testing against ? (Android version, CPU / make, memory, etc.)

[vustav]

I did a quick copy/paste on my lunchbreak like this:

And calling only that one from the button. No other changes in the project.

I'm at work right now but I'll adress the other questions when I get time!

[vustav]

But I can say right away that I've had this issue on two different computers with different specs, windows and linux, and it happens both on my phone and android studio emulator.

[vustav]

I got the crash eventually using the version in your new branch. But some serious mashing is needed. In my app more happens so the probability of it happening is just larger I guess.

JDK 17.0.7
Emulator in Android Studio (Hedgehog 2023.1.1)
CPU: Intel i7-3630QM (8) @ 3.400GHz
MEMORY: 15884MiB
nothing out of the ordinary when checking Profiler in AS

[igorski]

Hi @vustav thanks for the info, I have used the same AS version, JDK and created an emulator with low memory settings (in the hopes of forcing the garbage collector to work harder by also allocating extra memory during app start).

Could you pull the changes from the same branch, build the engine and see if there's a noticeable change in your apps stability ? I have made changes to the disposal routine which could indeed lead to a null pointer exception when cleaning up SampleEvents during playback.

[vustav]

Tried the new version and I'm afraid I'm still having the same crashes.

The emulator is a Pixel 4a, API 34 (Android 14.0 "UpsideDownCake"), x86_64. The phone is a Samsung s22+ with whatever version of android it's auto-updated to. Currently 14.

I really appreciate you doing this, really really do.

[igorski]

Alright, @vustav I doctored the engine to instantly deallocate memory on dispose (this to mimic the behaviour of a "forced" garbage collection from Java), this gave me a 100 % success rate of getting crashes.

I have added a new method to the engine (still in the event-crashes branch). Basically, you can now request execution of a function that is delayed until the engine reaches the idle phase beween renders (at worst, this is in the order of milliseconds), if the Sequencer is running. See the updated example activity and its use of the new MWEngine.executeWhenIdle method.

You can repeatedly pass (lambda) functions to executeWhenIdle but ideally you batch all your operations in a single call. In the example app this now leads to no crashes. Could you verify after a rebuild of the engine and wrapping your dispose calls inside executeWhenIdle() whether it also restores stability in your app ?