View Issue Details

IDProjectCategoryView StatusLast Update
0001664OpenMPTPlugins / VSTpublic2023-02-11 14:46
ReporterVad1m_1719 Assigned ToSaga Musix  
PriorityhighSeveritycrashReproducibilityhave not tried
Status resolvedResolutionfixed 
Platformx64OSWindowsOS Version10
Product VersionOpenMPT 1.31.00.* (old testing) 
Target VersionOpenMPT 1.31.01.00 / libopenmpt 0.7.0 (upgrade first)Fixed in VersionOpenMPT 1.31.01.00 / libopenmpt 0.7.0 (upgrade first) 
Summary0001664: Crash when loading plugins while song is playing
Description

https://cdn.discordapp.com/attachments/1067281758476828843/1073908350317760512/image.png I lost a module during cover to mptm (module is not saved) and OpenMPT crash files and dumps is not saved. I didn't know. Why I'm lost this work when crashes and NOT SAVED this module to OpenMPT Crash Files
[Module NOT restored after the major crash]

Additional Information
Faulting application name: OpenMPT.exe, version: 1.31.00.24, time stamp: 0x63e4e838
Faulting module name: OpenMPT.exe, version: 1.31.00.24, time stamp: 0x63e4e838
Exception code: 0xc0000409
Fault offset: 0x000000000055b079
Faulting process id: 0x391c
Faulting application start time: 0x01d93d692266fddc
Faulting application path: C:\Program Files\OpenMPT\bin\amd64\OpenMPT.exe
Faulting module path: C:\Program Files\OpenMPT\bin\amd64\OpenMPT.exe
Report Id: 6a68708d-e447-4749-bae5-7fa3d75987b2
Faulting package full name: 
Faulting package-relative application ID: 
TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)18724

Activities

Saga Musix

Saga Musix

2023-02-11 11:40

administrator   ~0005546

0xc0000409 would be a stack corruption, but I cannot reproduce that here. Was the plugin that you inserted a bridged plugin like shown in the screenshot?

You could try to reproduce the issue by using procdump to generate a memory dump: https://wiki.openmpt.org/Manual:_Frequently_Asked_Questions#OpenMPT_crashed_but_did_not_create_a_memory_dump

Saga Musix

Saga Musix

2023-02-11 14:11

administrator   ~0005547

I think I see what's going on now, the memory dump is no longer required. An important aspect is that audio playback must be running while the plugin is loaded.

This broke in r17406. Plugin instances are now inserted into the SNDMIXPLUGIN struct at the start of creation time, not after the plugin set-up is completed. Previously this was thread-safe because the plugin object was not known yet to the audio thread, so initializing it on the main thread without any locks was safe and the song could continue playing in the background. Now the plugin is inserted into the plugin list, and while it is being set up, the audio thread may already attempt to use the plugin. This could be solved by extending the global lock, but this would mean that audio processing is interrupted while the plugin is loaded. More complex VST plugins can take quite a few seconds to load, so I think this option is not so user-friendly as audio playback is not possible during that time.

@manx I would suggest to partially rework r17406 to make sure that m_pMixStruct->pMixPlugin = this; is not run in the IMixPlugin constructor. It either has to happen at the end of individual plugin constructors as before (technically not thread-safe but on relevant platforms the pointer update is atomic), or the call sites that create plugins need to take care of it as required (CVstPluginManager::CreateMixPlugin and mod2midi.cpp). The latter option requires fewer changes.

Saga Musix

Saga Musix

2023-02-11 14:18

administrator   ~0005548

Potential fix using the second option

Fix-Plugin-Insertion.patch (2,188 bytes)   
Index: mptrack/mod2midi.cpp
===================================================================
--- mptrack/mod2midi.cpp	(revision 18727)
+++ mptrack/mod2midi.cpp	(working copy)
@@ -456,6 +456,7 @@
 
 			m_tracks.reserve(m_sndFile.GetNumInstruments() + 1);
 			MidiTrack &tempoTrack = *(new MidiTrack(m_plugFactory, m_sndFile, m_songLength, tempoTrackPlugin, nullptr, mpt::ToUnicode(m_sndFile.GetCharsetInternal(), m_sndFile.m_songName), nullptr, overlappingInstruments));
+			tempoTrackPlugin.pMixPlugin = &tempoTrack;
 			tempoTrack.WriteString(kText, mpt::ToUnicode(m_sndFile.GetCharsetInternal(), m_sndFile.m_songMessage.GetString()));
 			tempoTrack.WriteString(kCopyright, m_sndFile.m_songArtist);
 			m_tracks.push_back(&tempoTrack);
Index: soundlib/plugins/PluginManager.cpp
===================================================================
--- soundlib/plugins/PluginManager.cpp	(revision 18727)
+++ soundlib/plugins/PluginManager.cpp	(working copy)
@@ -737,6 +737,8 @@
 		{
 			pFound->InsertPluginInstanceIntoList(*plugin);
 		}
+		CriticalSection cs;
+		mixPlugin.pMixPlugin = plugin;
 		return plugin != nullptr;
 	}
 
@@ -785,8 +787,6 @@
 
 		if(pEffect != nullptr && pEffect->dispatcher != nullptr && pEffect->magic == Vst::kEffectMagic)
 		{
-			validPlugin = true;
-
 			GetPluginInformation(maskCrashes, pEffect, *pFound);
 
 			// Update cached information
@@ -797,10 +797,9 @@
 			{
 				pFound->InsertPluginInstanceIntoList(*pVstPlug);
 			}
-			if(pVstPlug == nullptr)
-			{
-				validPlugin = false;
-			}
+			validPlugin = (pVstPlug != nullptr);
+			CriticalSection cs;
+			mixPlugin.pMixPlugin = pVstPlug;
 		}
 
 		if(!validPlugin)
Index: soundlib/plugins/PlugInterface.cpp
===================================================================
--- soundlib/plugins/PlugInterface.cpp	(revision 18727)
+++ soundlib/plugins/PlugInterface.cpp	(working copy)
@@ -54,7 +54,6 @@
 	, m_pMixStruct(&mixStruct)
 {
 	m_SndFile.m_loadedPlugins++;
-	m_pMixStruct->pMixPlugin = this;
 	m_MixState.pMixBuffer = mpt::align_bytes<8, MIXBUFFERSIZE * 2>(m_MixBuffer);
 	while(m_pMixStruct != &(m_SndFile.m_MixPlugins[m_nSlot]) && m_nSlot < MAX_MIXPLUGINS - 1)
 	{
Fix-Plugin-Insertion.patch (2,188 bytes)   
Vad1m_1719

Vad1m_1719

2023-02-11 14:31

reporter   ~0005549

ok. I'm waiting to bot build, after the build, updating to the latest

manx

manx

2023-02-11 14:42

administrator   ~0005550

Yeah, patch looks fine to me.

Saga Musix

Saga Musix

2023-02-11 14:46

administrator   ~0005551

Fixed in r18728.

Issue History

Date Modified Username Field Change
2023-02-11 10:19 Vad1m_1719 New Issue
2023-02-11 11:40 Saga Musix Note Added: 0005546
2023-02-11 13:52 Saga Musix Assigned To => Saga Musix
2023-02-11 13:52 Saga Musix Status new => assigned
2023-02-11 14:11 Saga Musix Note Added: 0005547
2023-02-11 14:18 Saga Musix Note Added: 0005548
2023-02-11 14:18 Saga Musix File Added: Fix-Plugin-Insertion.patch
2023-02-11 14:20 Saga Musix Summary Tracker was crashing when putting plugin into FX 8 => Crash when loading plugins while song is playing
2023-02-11 14:31 Vad1m_1719 Note Added: 0005549
2023-02-11 14:42 manx Note Added: 0005550
2023-02-11 14:46 Saga Musix Note Added: 0005551
2023-02-11 14:46 Saga Musix Status assigned => resolved
2023-02-11 14:46 Saga Musix Resolution open => fixed
2023-02-11 14:46 Saga Musix Fixed in Version => OpenMPT 1.31.01.00 / libopenmpt 0.7.0 (upgrade first)
2023-02-11 14:46 Saga Musix Target Version => OpenMPT 1.31.01.00 / libopenmpt 0.7.0 (upgrade first)