Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
remaininlight opened this issue Jul 24, 2024 · 3 comments
Open

Comments

@remaininlight
Copy link

remaininlight commented Jul 24, 2024

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
Copy link
Author

remaininlight commented Jul 24, 2024

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);
    }
    ```

@remaininlight remaininlight changed the title Using synchronous set to false in Patch::loadPatch causes threading crashes Using synchronous set to false in Patch::loadPatch causes crashes Jul 24, 2024
@cesaref
Copy link
Contributor

cesaref commented Jul 25, 2024

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

@remaininlight
Copy link
Author

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);
        });
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants