Skip to content

Commit

Permalink
Fixes a Segmentation Fault bug
Browse files Browse the repository at this point in the history
	There was a segmentation fault bug that happened if a project file was loaded and one of the tracks had a Knob set to a different value. That would trigger a processCCEvent call while the Track was still being loaded, which then would call processInEvent. At the moment processInEvent was called, m_instrument was still NULL, so instrument()->handleMidiEvent caused LMMS to crash. To fix this two measures were taken:
	1) Added a sanity check on processInEvent, so it only calls handleMidiEvent if m_instrument is not NULL.
	2) During the track being loaded (InstrumentTrack::loadTrackSpecificSettings), we first set m_midiCCEnable to false, so the Knob value changes don't trigger processInEvent calls, then load all knob models and finally load the correct value of m_midiCCEnable.

	Also added a comment on the InstrumentTrack constructor explaining why the MIDI CC models need to be created before setName (to avoid the position being changed later and a mysterious Segmentation Fault being caused by consequence).
  • Loading branch information
IanCaio committed Sep 9, 2020
1 parent d18b25b commit 6abd130
Showing 1 changed file with 16 additions and 4 deletions.
20 changes: 16 additions & 4 deletions src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ InstrumentTrack::InstrumentTrack( TrackContainer* tc ) :
}


// IMPORTANT NOTE: We have to create the MIDI CC models before calling setName, because the latter
// will trigger a trackRenamed signal on the track container, which will then call
// MidiCCRackView::updateTracksComboBox, which itself calls MidiCCRackView::updateKnobsModels. That
// last method expects the Tracks CC Enable and Knob models to be created already.

// Initialize the m_midiCCEnabled variable, but it's actually going to be connected
// to a LedButton
m_midiCCEnable = new BoolModel( false, NULL, "Enable/Disable MIDI CC" );
Expand Down Expand Up @@ -408,7 +413,9 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti
break;
}

if( eventHandled == false && instrument()->handleMidiEvent( event, time, offset ) == false )
// If the event wasn't handled, check if there's a loaded instrument and if so send the
// event to it. If it returns false means the instrument didn't handle the event, so we trigger a warning.
if( eventHandled == false && instrument() && instrument()->handleMidiEvent( event, time, offset ) == false )
{
qWarning( "InstrumentTrack: unhandled MIDI event %d", event.type() );
}
Expand Down Expand Up @@ -848,12 +855,13 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement
m_baseNoteModel.loadSettings( thisElement, "basenote" );
m_useMasterPitchModel.loadSettings( thisElement, "usemasterpitch");

// Load MIDI CC stuff
m_midiCCEnable->loadSettings( thisElement, "enablecc" );

// clear effect-chain just in case we load an old preset without FX-data
m_audioPort.effects()->clear();

// We set MIDI CC enable to false so the knobs don't trigger MIDI CC events while
// they are being loaded. After all knobs are loaded we load the right value of m_midiCCEnable.
m_midiCCEnable->setValue(false);

QDomNode node = thisElement.firstChild();
while( !node.isNull() )
{
Expand Down Expand Up @@ -925,6 +933,10 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement
}
node = node.nextSibling();
}

// Load the right value of m_midiCCEnable
m_midiCCEnable->loadSettings( thisElement, "enablecc" );

updatePitchRange();
unlock();
}
Expand Down

0 comments on commit 6abd130

Please sign in to comment.