Skip to content

Fix script console crashing when reopened.

Tuomas Nurmi requested to merge nurmi/amarok:scriptingImprovements into master

When script console is opened for the second time, it crashes. I.e. if enabled, it is opened first time at program startup. If closed, script console will open again when one opens Amarok settings and applies them with Apply or OK, causing also a Segmentation fault in QJSEngine::newQObject(QObject*). This originates from MetaTrackPrototype::init (and next one would come from TrackSetExporter::init), where if (s_wrapper == nullptr) s_wrapper = new MetaTrackPrototypeWrapper( engine ); QJSValue scriptObj = engine->newQObject( s_wrapper ); is called. However, as the MetaTrackPrototypeWrapper is parented to engine on creation, deletion of engine (e.g. when closing the script console) apparently causes s_wrapper to get deleted, too. Or if it isn't parented, it gets probably cleaned away when the QJSEngine shuts down, assumedly because it has been wrapped to QJSEngine with engine->newQObject(). And even if it didn't get deleted, the trackCtor function returns m_engine->newQObject(...), engine being already deleted, so seems that a new instance should be used for each new engine, instead of a static wrapper.

There's still another aspect not yet addressed by this, though: On new script console window creation, qRegisterMetaType()'s and QMetaType::registerConverter()'s, that apparently should only be called once, are called again, resulting in such debug output: Type conversion already registered from type Collections::Collection* to type QJSValue Type conversion already registered from type QJSValue to type Collections::Collection*

While these don't seem to be dangerous and could easily be safeguarded against by e.g. adding some static s_QMetaTypeInitialized variables for each, there seems to be a catch: the global registerConverters from C++ classes to QJSValues are created with local QJSEngine *engine as parameter, which is then used by toScriptValue() to do the JS Object creation. I'd imagine this fails if the engine used for creation is not the same as the Object's target engine, and crashes if the engine has been deleted since, which seems to happen at at least closing of script window.

I haven't yet encountered these errors/crashes, however, and I'm not quite sure when the registerConverters actually get used. I guess one solution could be registering converters to be from e.g. QPairMeta::TrackPtr,QJSEngine* to QJSValue>, and forwarding the current QJSEngine* to toScriptValue when doing any conversions.

I'm not completely sure if this is the best path anyhow (using QJSEngine::newQMetaObject looks somewhat promising approach, too), but for now, I think it is a right step, and I will probably gain better understanding of the conversions and everything script-related now that I can test the scripting tools a bit more easily as Amarok doesn't crash every time I want to re-open the console.

BUG: 477180

Also disable script updater & KNewStuff button from settings. To be reviewed sometime after 3.0 after improvements to the system and maybe the ability to display content warning in KNewStuff UI.

Edited by Tuomas Nurmi

Merge request reports