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 ?

[vustav]

I'm afraid I'm still getting the same crashses.

And I realize now it's not only when the sequencer is playing. If I delete events when stopped it often crashes on the last or first step when playing again, when the sequencer is about to restart from step 0 or whatever the correct term for it is. When the sequencer is running it can happen on any step.

Still stable when keeping the events. Having some solid experience when it comes to my own coding skills and my tendencies to do things the wrong way I can only assume I'm doing something stupid somewhere and this is entirely on me. Since keeping the events isn't something that always fills up from normal use but only happens when you actively do something (deleting steps), I'm not too concerned by it and so far it hasn't been a problem at all. I've tested creating 50k events and just deleting them and it doesn't impact performance in any meaningful way, and normal usage won't get you anywhere near that. Really appreciate you being so helpful witht his!

[igorski]

I'm afraid I'm still getting the same crashses.

Even when wrapping inside executeWhenIdle ? :(

I could help by looking at your implementation if you don't mind sharing access to your repo (if not, that is understandable).

[vustav]

Of course! I'll set you up during the day.

[vustav]

I think I've invited you now.

It's very messy but the basics are:

DrumPattern -> DrumTrack -> Step -> StepEventsManager -> Sub

A drum can consists of several "sounds", meaning a substep can have several events playing at the same time, and that's the reason for stepEventsManager.

Every track has a soundManager, and every soundManager has two SoundSourceManagers, one for samples and one for oscillators, meaning every track has two instruments, one for SampleEvents and one for SynthEvents (and two more for live events). For every SoundSource the Step-objects will create a StepEventsManager, and for every sub the eventsManager will create two Events, one sample and one oscillator. Only one of them is enabled at a time, the reason for this being to avoid deleting events when switching from sample to oscillator. I've had troubles deleting events for a long time and this is a way of minimizing that. I hope you'll understand better when you use the app and see for yourself.

Deletions takes place in Deleter. Lots of uncommented code there from me trying different ways of deleting events but deleteEvent(BaseAudioEvent event) is where that happens. Clearing the array is disabled by default but can be turned on in options (many of them does nothing right now).

To trigger a crash make sure "DO NOT KEEP REMOVED EVENTS IN ARRAY" is ticked in options and remove some steps (the "-") in the top right, followed by randomizing a new pattern (the flashing rnd-button bottom right), and keep doing that until it crashes. If the engine is running it will happen sooner or later, if its paused it will probably crash when you play again. These options are not saved so you have to tick it every time you start the app.

Removing a step fires DrumPattern.removeStep and from there you can follow what happens.