Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#7558 closed bug (fixed)

BSynth destructor never called by BMidiSynth

Reported by: Pete Owned by: korli
Priority: normal Milestone: R1
Component: Kits/Midi Kit Version: R1/alpha2
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

MidiPlayer and other apps that use BMidiSynth were crashing on exit. Usually the MediaEventLooper was trying to handle an event after the data had gone away. In addition Cortex and the Audio Mixer panel would never delete their representations of Synth nodes. A bit of tracing showed that the BSynth destructor was never called.

Fixed in the attached patch. delete is invoked on be_synth when all clients have gone.

Attachments (1)

midisynth.diff​ (337 bytes ) - added by Pete 13 years ago.
style fix of original patch

Download all attachments as: .zip

Change History (14)

comment:1 by Pete, 13 years ago

patch: 0 → 1

comment:2 by korli, 13 years ago

OK, please fix the coding style.

by Pete, 13 years ago

Attachment: midisynth.diff​ added

style fix of original patch

in reply to:  2 comment:3 by Pete, 13 years ago

Replying to korli:

OK, please fix the coding style.

Sigh... one line, and I can get it wrong! (:-/)

Corrected now.

BTW, I noticed that you looked at my USB_MIDI stuff (#4463) months ago, and reassigned it to phoudoin, but Philippe has never looked at it. Any chance of getting it committed? (It's essential for me, as the 2x2 is my standard interface to Haiku.)

Last edited 13 years ago by Pete (previous) (diff)

comment:4 by korli, 13 years ago

Applied in hrev41717. "be_synth = NULL" was added for good measure.

comment:5 by korli, 13 years ago

Resolution: → fixed
Status: new → closed

in reply to:  4 ; comment:6 by Pete, 13 years ago

Replying to korli:

Applied in hrev41717. "be_synth = NULL" was added for good measure.

Thanks. (The nulling is probably a wise move!)

in reply to:  6 comment:7 by Pete, 13 years ago

Replying to Pete:

Replying to korli:

Applied in hrev41717. "be_synth = NULL" was added for good measure.

Thanks. (The nulling is probably a wise move!)

Oops!! NO! Nulling be_synth causes a prompt, certain crash. Apparently it is accessed later somewhere.

The reason for my original patch was that the BSynth destructor stops the BMediaEventLooper before returning. (It does not crash with just that addition.)

Last edited 13 years ago by Pete (previous) (diff)

comment:8 by Pete, 13 years ago

Resolution: fixed
Status: closed → reopened

comment:9 by korli, 13 years ago

OK It might the reason why it was not deleted before hrev41717. Would you prefer we revert this patch?

in reply to:  9 comment:10 by Pete, 13 years ago

Replying to korli:

Would you prefer we revert this patch?

Not the whole patch -- just remove the 'be_synth = NULL;' statement.

Leaving it out doesn't actually cause any problems. The absence of the 'delete', though, does cause the crashes and other problems mentioned originally, so it has to be there.

I'll look at what is actually referencing be_synth later, but I think it's better to have things solid even if not perfectly understood!

comment:11 by Pete, 13 years ago

Dohh -- my bad. I applied the added line wrongly!!

There is nothing wrong with the patch as committed. Please close this ticket again as fixed.

Many apologies (:-()

comment:12 by diver, 13 years ago

Resolution: → fixed
Status: reopened → closed

comment:13 by korli, 13 years ago

Thanks for the feedback Pete!

Note: See TracTickets for help on using tickets.