https://bugs.kde.org/show_bug.cgi?id=477180

Tuomas Nurmi <tuo...@norsumanageri.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Latest Commit|                            |https://invent.kde.org/mult
                   |                            |imedia/amarok/-/commit/7288
                   |                            |78aaf2d81bb4fdbfce2b982ced3
                   |                            |4f07f5692
         Resolution|---                         |FIXED

--- Comment #2 from Tuomas Nurmi <tuo...@norsumanageri.org> ---
Git commit 728878aaf2d81bb4fdbfce2b982ced34f07f5692 by Tuomas Nurmi.
Committed on 07/04/2024 at 08:16.
Pushed by nurmi into branch 'master'.

Fix script console crashing when reopened.

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. QPair<Meta::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.

M  +1    -1    ChangeLog
M  +2    -6    src/scripting/scriptengine/exporters/MetaTypeExporter.cpp
M  +0    -1    src/scripting/scriptengine/exporters/MetaTypeExporter.h
M  +2    -9    src/scripting/scriptengine/exporters/ScriptableBiasExporter.cpp
M  +0    -3    src/scripting/scriptengine/exporters/ScriptableBiasExporter.h

https://invent.kde.org/multimedia/amarok/-/commit/728878aaf2d81bb4fdbfce2b982ced34f07f5692

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to