search

juce-framework / JUCE

JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, LV2 and AAX audio plug-ins.
https://juce.com
Other
6.3k stars 1.67k forks source link

[Bug]: (JUCE8) variant.isVoid() not handled in VariantConverter fromVar() #1399

Closed benkuper closed 1 day ago

benkuper commented 1 week ago

Detailed steps on how to reproduce the bug

Using the new QuickJS JS Engine, calling from a script a C function that returns a var() (variant Void) leads to an assert because this is not a handled case in the function :

struct VariantConverter<choc::value::Value>
{
    static choc::value::Value fromVar (const var& variant)

Or is var() as return type not allowed / good practice ? It has been working great with the [old] engine

What is the expected behaviour?

The end of the function 

        if (variant.isUndefined())
            return {};

could be 

        if (variant.isUndefined() || variant.isVoid())
            return {};

Operating systems

Windows

What versions of the operating systems?

11

Architectures

64-bit

Stacktrace

No response

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

No response

Testing on the develop branch

The bug is present on the develop branch

Code of Conduct

reuk commented 1 day ago

I believe this issue is resolved as a side-effect of this change: https://github.com/juce-framework/JUCE/commit/606a7bc552b0737d2dee8feca3a203f167c33662

Please try it out and let us know if you encounter further issues.

benkuper commented 23 hours ago

so I don't get the same error :) Now everything seems ok, except when I try to call engine->getRootObjectProperties() after execute(), then I get : 

Exception thrown: read access violation.
this was 0xFFFFFFFFFFFFFFFF.

Stack trace

Chataigne.exe!juce::var::isString() Line 559    C++
Chataigne.exe!juce::JSObject::Impl::getProperties() Line 940    C++
Chataigne.exe!juce::JSObject::getProperties() Line 1076 C++
Chataigne.exe!juce::JavascriptEngine::getRootObjectProperties() Line 811    C++

thought it was because of some regiterNativeObjects but it's actually not. For testing purpose, I removed everything extra and i'm left with only creating the Engine, executing the script (which is an empty string for testing) and then calling getRootObjectProperties

If you need more info I can give you more (win 11, x64)

benkuper commented 23 hours ago

Went a bit further to ease your testing : I removed my environment from the equation, taking the JavascriptDemo to test. Already just running it without any modification leads to an (unrelated I presume) jassert in juceToQuickJs function (juce_Javascript.cpp line 183) because v is Undefined (and then is not handled before the end of function)

Otherwise the script runs fine. BUT ! Added engine.getRootObjectProperties() after the execute() function leads to the same crash as what I had in my software.