cmajor-lang / cmajor

The Cmajor public repository
https://cmajor.dev
Other
523 stars 31 forks source link

Using synchronous set to false in Patch::loadPatch causes crashes #74

Open remaininlight opened 1 month ago

remaininlight commented 1 month ago

When using synchronous set to false in Patch::loadPatch (https://github.com/cmajor-lang/cmajor/blob/main/include/cmajor/helpers/cmaj_Patch.h#L1975) I get these crashes about 50% of the time when I load a new patch, more if it's a bigger patch:

AudioCalc (41)#0    0x0000000107c881ac in pthread_mutex_unlock ()
#1  0x0000000193dde760 in std::__1::mutex::unlock() ()
#2  0x000000034a1626f0 in cmaj::Patch::PatchRenderer::endProcessBlock() at /Users/user/Bounce/groove-engine/native/dependencies/cmajor/include/cmajor/helpers/cmaj_Patch.h:1547
#3  0x000000034a15b9dc in cmaj::Patch::endChunkedProcess() at /Users/user/Bounce/groove-engine/native/dependencies/cmajor/include/cmajor/helpers/cmaj_Patch.h:2360
#4  0x000000034a159d1c in cmaj::Patch::process(float* const*, unsigned int, std::__1::function<void (unsigned int, choc::midi::Message<choc::midi::MIDIMessageStorageView>)> const&) at /Users/user/Bounce/groove-engine/native/dependencies/cmajor/include/cmajor/helpers/cmaj_Patch.h:2334
#5  0x0000000349fe8f78 in cmaj::plugin::JUCEPluginBase<cmaj::plugin::JITLoaderPlugin>::processBlock(juce::AudioBuffer<float>&, juce::MidiBuffer&) at /Users/user/Bounce/groove-engine/native/dependencies/cmajor/include/cmajor/helpers/cmaj_JUCEPlugin.h:304
#6  0x0000000349f4bd70 in PatchManager::processBlock(juce::AudioBuffer<float>&, juce::MidiBuffer&) at /Users/user/Bounce/groove-engine/native/Source/PatchManager.h:344
#7  0x0000000349f4ba0c in GrooveEngineAudioProcessor::processBlock(juce::AudioBuffer<float>&, juce::MidiBuffer&) at /Users/user/Bounce/groove-engine/native/Source/PluginProcessor.cpp:345
#8  0x0000000349eecebc in void juce::JuceVST3Component::processAudio<float>(Steinberg::Vst::ProcessData&) at /Users/user/Bounce/groove-engine/native/dependencies/JUCE/modules/juce_audio_plugin_client/juce_audio_plugin_client_VST3.cpp:3704
#9  0x0000000349ebbde4 in juce::JuceVST3Component::process(Steinberg::Vst::ProcessData&) at /Users/user/Bounce/groove-engine/native/dependencies/JUCE/modules/juce_audio_plugin_client/juce_audio_plugin_client_VST3.cpp:3582
#10 0x000000010270dd74 in ___lldb_unnamed_symbol220141 ()
#11 0x0000000102707618 in ___lldb_unnamed_symbol220026 ()
#12 0x000000010270710c in ___lldb_unnamed_symbol220025 ()
#13 0x00000001026fe784 in ___lldb_unnamed_symbol219860 ()
#14 0x000000010151195c in ___lldb_unnamed_symbol133082 ()
#15 0x000000010152cc8c in ___lldb_unnamed_symbol133842 ()
#16 0x0000000101514e4c in ___lldb_unnamed_symbol133178 ()
#17 0x00000001015150c4 in ___lldb_unnamed_symbol133179 ()
#18 0x0000000100d0c174 in ___lldb_unnamed_symbol90425 ()
#19 0x0000000100d10fe0 in ___lldb_unnamed_symbol90557 ()
#20 0x0000000107c815c0 in _pthread_start ()

I think there might be something going on with multiple threads attempting to access the processLock mutex on Patch::PatchRenderer here (https://github.com/cmajor-lang/cmajor/blob/main/include/cmajor/helpers/cmaj_Patch.h#L1546). Maybe the Builder thread and the Audio thread?

Also, the renderer on the Patch is a nullptr at the point of the crash

Even when I check if plugin->patch->renderer is not a nullptr before calling processBlock I still get this error. It looks like maybe the Patch::setNewRenderer (https://github.com/cmajor-lang/cmajor/blob/main/include/cmajor/helpers/cmaj_Patch.h#L2568) is setting the render before it is initialised fully? Then maybe thread interference is happening here?

Happy to provide any more information, I've been chasing this one round for weeks (it's crashing my DAW often when I reload a patch) and can't work out how to fix it

Any help/pointers much appreciated :)

remaininlight commented 1 month ago

Here's how I'm loading the patch on a PatchManager class:

bool loadPatch (const std::string& codeIn, const choc::value::ValueView& paramsIn)
    {
        parameterList = {};

        customCode = codeIn;

        jassert (plugin != nullptr);
        jassert (plugin->patch != nullptr);

        cmaj::PatchManifest manifest;

        initialiseManifest (manifest, std::filesystem::path ("Main/Main.cmajorpatch"));

        cmaj::Patch::LoadParams params;

        for (const auto& param : paramsIn)
        {
            std::string messageJsonString = choc::json::toString(param);
            auto type = param["value"].getType();

            try {
                params.parameterValues[std::string{param["name"].getString()}] = param["value"].get<float>();
            } catch (const choc::value::Error& e) {
                juce::Logger::writeToLog("Error casting value to float: " + std::string(e.what()));
            }
        }

        try
        {
            params.manifest = std::move (manifest);
            params.manifest.reload();
        }
        catch (const choc::json::ParseError& e)
        {
            juce::Logger::writeToLog("Parse Error: " + std::string(e.what()) + " in manifest file at line: " + std::to_string(e.lineAndColumn.line) + ", column: " + std::to_string(e.lineAndColumn.column));
            return false;
        }
        catch (const std::runtime_error& e)
        {
            juce::Logger::writeToLog("Runtime Error: " + std::string(e.what()));
            return false;
        }

        bool synchronous = false;
        bool isPlayable = plugin->patch->loadPatch(params, synchronous);

        logPluginDescription();
        return isPlayable;
    }

On the same PatchManager class processBlock looks like this:

void processBlock (juce::AudioBuffer<float>& buffer, juce::MidiBuffer& midiMessages)
    {
        if (plugin == nullptr)
        {
            juce::Logger::writeToLog("Error: plugin is null in processBlock");
            return;
        }

        if (plugin->patch == nullptr)
        {
            juce::Logger::writeToLog("Error: plugin->patch is null in processBlock");
            return;
        } 

        if (plugin->patch->renderer == nullptr)
        {
            juce::Logger::writeToLog("Error: plugin->patch->renderer is null in processBlock");
            return;
        }

        plugin->processBlock (buffer, midiMessages);
    }
cesaref commented 1 month ago

Ok, i'll take a look. I've been fiddling with this recently and i've probably managed to break it without realising...

remaininlight commented 1 month ago

Thanks for taking a look Chez!

Here's how I'm initialising the plugin - initialisePlugin() is a method on my PatchManager, called once. My code for running the plugin is pretty cobbled together (any pointers appreciated!). Here it is in case that's playing any part in the crashes, though it used to work

void initialisePlugin()
    {
        auto patch = std::make_unique<cmaj::Patch> ();

        patch->cache = cache;

        plugin = std::make_unique<cmaj::plugin::JITLoaderPlugin>(std::move (patch));

        plugin->patchChangeCallback = std::move (changeCallback);

        plugin->handleConsoleMessage = +[] (const char* message)
        {
            juce::Logger::writeToLog (message);
        };

        plugin->patch->createEngine = [&] {
            auto availableTypes = cmaj::Engine::getAvailableEngineTypes();
            auto engine = cmaj::Engine::create();
            return engine;
        };

        plugin->patch->statusChanged = std::move (statusChangedCallback);

        cmaj::enableWebViewPatchWorker(*plugin->patch.get());

        patchView = std::make_unique<GroovePatchView>( // Inherited from cmaj::PatchView
            *(plugin->patch.get()),
            [&](const choc::value::ValueView& message)
            {
                juce::Logger::writeToLog("GroovePatchView::sendMessage content: " + choc::json::toString(message, true));
                engineMessageCallback(message);
        });
    }