Fixed possible crash after assigning to tiled.activeAsset

When assigning a new map or tileset created from a script to tiled.activeAsset,
the necessary signals were not connected. This broke the signals from
EditableMap and it also caused certain ownership transfers from JS to C++ to
never take place. The latter could in turn lead to dangling pointers to objects
after they were deleted by the garbage collector.

Thanks to @eishiya for discovering this issue!

NEWS.md

@@ -19,6 +19,7 @@
 * AutoMapping: Ignore empty outputs per-rule (#3523)
 * AutoMapping: Always apply output sets with empty index
 * Windows: Fixed the support for WebP images (updated to Qt 6.6.1, #3661)
+* Fixed possible crash after assigning to tiled.activeAsset
 * Fixed the option to resolve properties on export to also resolve class members (#3411, #3315)
 * Fixed terrain tool behavior and terrain overlays after changing terrain set type (#3204, #3260)
 * Fixed mouse handling issue when zooming while painting (#3863)

src/tiled/changemapobject.cpp

@@ -100,9 +100,7 @@ static QList<MapObject*> objectList(const QVector<MapObjectCell> &changes)
 
 void ChangeMapObjectCells::swap()
 {
-    for (int i = 0; i < mChanges.size(); ++i) {
-        MapObjectCell &change = mChanges[i];
-
+    for (auto &change : mChanges) {
         auto cell = change.object->cell();
         change.object->setCell(change.cell);
         change.cell = cell;

src/tiled/editableasset.cpp

@@ -28,14 +28,9 @@
 
 namespace Tiled {
 
-EditableAsset::EditableAsset(Document *document, Object *object, QObject *parent)
+EditableAsset::EditableAsset(Object *object, QObject *parent)
     : EditableObject(this, object, parent)
-    , mDocument(document)
 {
-    if (document) {
-        connect(document, &Document::modifiedChanged,
-                this, &EditableAsset::modifiedChanged);
-    }
 }
 
 QString EditableAsset::fileName() const
@@ -126,6 +121,22 @@ void EditableAsset::redo()
         ScriptManager::instance().throwError(QCoreApplication::translate("Script Errors", "Undo system not available for this asset"));
 }
 
+void EditableAsset::setDocument(Document *document)
+{
+    if (mDocument == document)
+        return;
+
+    if (mDocument)
+        mDocument->disconnect(this);
+
+    if (document) {
+        connect(document, &Document::modifiedChanged,
+                this, &EditableAsset::modifiedChanged);
+    }
+
+    mDocument = document;
+}
+
 } // namespace Tiled
 
 #include "moc_editableasset.cpp"

src/tiled/editableasset.h

@@ -57,7 +57,7 @@ class EditableAsset : public EditableObject
     Q_PROPERTY(AssetType::Value assetType READ assetType CONSTANT)
 
 public:
-    EditableAsset(Document *document, Object *object, QObject *parent = nullptr);
+    EditableAsset(Object *object, QObject *parent = nullptr);
 
     QString fileName() const;
     bool isReadOnly() const override = 0;
@@ -88,11 +88,13 @@ public slots:
     void modifiedChanged();
     void fileNameChanged(const QString &fileName, const QString &oldFileName);
 
+protected:
+    virtual void setDocument(Document *document);
+
 private:
     friend class Document;
-    void setDocument(Document *document);
 
-    Document *mDocument;
+    Document *mDocument = nullptr;
 };
 
 
@@ -101,11 +103,6 @@ inline Document *EditableAsset::document() const
     return mDocument;
 }
 
-inline void EditableAsset::setDocument(Document *document)
-{
-    mDocument = document;
-}
-
 } // namespace Tiled
 
 Q_DECLARE_METATYPE(Tiled::EditableAsset*)

src/tiled/editablemap.cpp

@@ -49,25 +49,16 @@
 namespace Tiled {
 
 EditableMap::EditableMap(QObject *parent)
-    : EditableAsset(nullptr, new Map(), parent)
+    : EditableAsset(new Map(), parent)
 {
     mDetachedMap.reset(map());
 }
 
 EditableMap::EditableMap(MapDocument *mapDocument, QObject *parent)
-    : EditableAsset(mapDocument, mapDocument->map(), parent)
+    : EditableAsset(mapDocument->map(), parent)
     , mSelectedArea(new EditableSelectedArea(mapDocument, this))
 {
-    connect(mapDocument, &Document::fileNameChanged, this, &EditableAsset::fileNameChanged);
-    connect(mapDocument, &Document::changed, this, &EditableMap::documentChanged);
-    connect(mapDocument, &MapDocument::layerAdded, this, &EditableMap::attachLayer);
-    connect(mapDocument, &MapDocument::layerRemoved, this, &EditableMap::detachLayer);
-
-    connect(mapDocument, &MapDocument::currentLayerChanged, this, &EditableMap::currentLayerChanged);
-    connect(mapDocument, &MapDocument::selectedLayersChanged, this, &EditableMap::selectedLayersChanged);
-    connect(mapDocument, &MapDocument::selectedObjectsChanged, this, &EditableMap::selectedObjectsChanged);
-
-    connect(mapDocument, &MapDocument::regionEdited, this, &EditableMap::onRegionEdited);
+    setDocument(mapDocument);
 }
 
 /**
@@ -76,14 +67,13 @@ EditableMap::EditableMap(MapDocument *mapDocument, QObject *parent)
  * The map's lifetime must exceed that of the EditableMap instance.
  */
 EditableMap::EditableMap(const Map *map, QObject *parent)
-    : EditableAsset(nullptr, const_cast<Map*>(map), parent)
+    : EditableAsset(const_cast<Map*>(map), parent)
     , mReadOnly(true)
-    , mSelectedArea(nullptr)
 {
 }
 
 EditableMap::EditableMap(std::unique_ptr<Map> map, QObject *parent)
-    : EditableAsset(nullptr, map.get(), parent)
+    : EditableAsset(map.get(), parent)
     , mDetachedMap(std::move(map))
 {
 }
@@ -449,7 +439,7 @@ void EditableMap::autoMap(const RegionValueType &region, const QString &rulesFil
         manager.autoMapRegion(region.region());
 }
 
-Tiled::ScriptImage *EditableMap::toImage(QSize size)
+Tiled::ScriptImage *EditableMap::toImage(QSize size) const
 {
     const MiniMapRenderer miniMapRenderer(map());
     const QSize imageSize = size.isValid() ? size : miniMapRenderer.mapSize();
@@ -682,6 +672,29 @@ QSharedPointer<Document> EditableMap::createDocument()
     return document;
 }
 
+void EditableMap::setDocument(Document *document)
+{
+    Q_ASSERT(!document || document->type() == Document::MapDocumentType);
+
+    if (this->document() == document)
+        return;
+
+    EditableAsset::setDocument(document);
+
+    if (auto doc = mapDocument()) {
+        connect(doc, &Document::fileNameChanged, this, &EditableAsset::fileNameChanged);
+        connect(doc, &Document::changed, this, &EditableMap::documentChanged);
+        connect(doc, &MapDocument::layerAdded, this, &EditableMap::attachLayer);
+        connect(doc, &MapDocument::layerRemoved, this, &EditableMap::detachLayer);
+
+        connect(doc, &MapDocument::currentLayerChanged, this, &EditableMap::currentLayerChanged);
+        connect(doc, &MapDocument::selectedLayersChanged, this, &EditableMap::selectedLayersChanged);
+        connect(doc, &MapDocument::selectedObjectsChanged, this, &EditableMap::selectedObjectsChanged);
+
+        connect(doc, &MapDocument::regionEdited, this, &EditableMap::onRegionEdited);
+    }
+}
+
 void EditableMap::documentChanged(const ChangeEvent &change)
 {
     switch (change.type) {

src/tiled/editablemap.h

@@ -163,7 +163,7 @@ class EditableMap final : public EditableAsset
     Q_INVOKABLE void autoMap(const QRectF &region, const QString &rulesFile = QString());
     Q_INVOKABLE void autoMap(const Tiled::RegionValueType &region, const QString &rulesFile = QString());
 
-    Q_INVOKABLE Tiled::ScriptImage *toImage(QSize size = QSize());
+    Q_INVOKABLE Tiled::ScriptImage *toImage(QSize size = QSize()) const;
 
     Q_INVOKABLE QPointF screenToTile(qreal x, qreal y) const;
     Q_INVOKABLE QPointF screenToTile(const QPointF &position) const;
@@ -209,6 +209,9 @@ class EditableMap final : public EditableAsset
 
     void regionEdited(const Tiled::RegionValueType &region, Tiled::EditableTileLayer *layer);
 
+protected:
+    void setDocument(Document *document) override;
+
 private:
     void documentChanged(const ChangeEvent &change);
 

src/tiled/editableproject.cpp

@@ -26,8 +26,9 @@
 namespace Tiled {
 
 EditableProject::EditableProject(ProjectDocument *projectDocument, QObject *parent)
-    : EditableAsset(projectDocument, &projectDocument->project(), parent)
+    : EditableAsset(&projectDocument->project(), parent)
 {
+    setDocument(projectDocument);
 }
 
 QString EditableProject::extensionsPath() const

src/tiled/editabletileset.cpp

@@ -36,29 +36,24 @@
 namespace Tiled {
 
 EditableTileset::EditableTileset(const QString &name, QObject *parent)
-    : EditableAsset(nullptr, nullptr, parent)
+    : EditableAsset(nullptr, parent)
     , mTileset(Tileset::create(name, 0, 0))
 {
     setObject(mTileset.data());
 }
 
 EditableTileset::EditableTileset(const Tileset *tileset, QObject *parent)
-    : EditableAsset(nullptr, const_cast<Tileset*>(tileset), parent)
+    : EditableAsset(const_cast<Tileset*>(tileset), parent)
     , mReadOnly(true)
     , mTileset(const_cast<Tileset*>(tileset)->sharedFromThis())    // keep alive
 {
 }
 
 EditableTileset::EditableTileset(TilesetDocument *tilesetDocument,
                                  QObject *parent)
-    : EditableAsset(tilesetDocument, tilesetDocument->tileset().data(), parent)
+    : EditableAsset(tilesetDocument->tileset().data(), parent)
 {
-    connect(tilesetDocument, &Document::fileNameChanged, this, &EditableAsset::fileNameChanged);
-    connect(tilesetDocument, &TilesetDocument::tilesAdded, this, &EditableTileset::attachTiles);
-    connect(tilesetDocument, &TilesetDocument::tilesRemoved, this, &EditableTileset::detachTiles);
-    connect(tilesetDocument, &TilesetDocument::tileObjectGroupChanged, this, &EditableTileset::tileObjectGroupChanged);
-    connect(tilesetDocument->wangSetModel(), &TilesetWangSetModel::wangSetAdded, this, &EditableTileset::wangSetAdded);
-    connect(tilesetDocument->wangSetModel(), &TilesetWangSetModel::wangSetRemoved, this, &EditableTileset::wangSetRemoved);
+    setDocument(tilesetDocument);
 }
 
 EditableTileset::~EditableTileset()
@@ -396,6 +391,25 @@ void EditableTileset::setBackgroundColor(const QColor &color)
         tileset()->setBackgroundColor(color);
 }
 
+void EditableTileset::setDocument(Document *document)
+{
+    Q_ASSERT(!document || document->type() == Document::TilesetDocumentType);
+
+    if (this->document() == document)
+        return;
+
+    EditableAsset::setDocument(document);
+
+    if (auto doc = tilesetDocument()) {
+        connect(doc, &Document::fileNameChanged, this, &EditableAsset::fileNameChanged);
+        connect(doc, &TilesetDocument::tilesAdded, this, &EditableTileset::attachTiles);
+        connect(doc, &TilesetDocument::tilesRemoved, this, &EditableTileset::detachTiles);
+        connect(doc, &TilesetDocument::tileObjectGroupChanged, this, &EditableTileset::tileObjectGroupChanged);
+        connect(doc->wangSetModel(), &TilesetWangSetModel::wangSetAdded, this, &EditableTileset::wangSetAdded);
+        connect(doc->wangSetModel(), &TilesetWangSetModel::wangSetRemoved, this, &EditableTileset::wangSetRemoved);
+    }
+}
+
 bool EditableTileset::tilesFromEditables(const QList<QObject *> &editableTiles, QList<Tile*> &tiles)
 {
     for (QObject *tileObject : editableTiles) {

src/tiled/editabletileset.h

@@ -30,7 +30,7 @@ class EditableWangSet;
 class ScriptImage;
 class TilesetDocument;
 
-class EditableTileset : public EditableAsset
+class EditableTileset final : public EditableAsset
 {
     Q_OBJECT
 
@@ -173,6 +173,9 @@ public slots:
     void setTransparentColor(const QColor &color);
     void setBackgroundColor(const QColor &color);
 
+protected:
+    void setDocument(Document *document) override;
+
 private:
     bool tilesFromEditables(const QList<QObject*> &editableTiles, QList<Tile *> &tiles);
 

src/tiled/editableworld.cpp

@@ -31,9 +31,10 @@
 namespace Tiled {
 
 EditableWorld::EditableWorld(WorldDocument *worldDocument, QObject *parent)
-    : EditableAsset(worldDocument, nullptr, parent)
+    : EditableAsset(nullptr, parent)
 {
     setObject(WorldManager::instance().worlds().value(worldDocument->fileName()));
+    setDocument(worldDocument);
 }
 
 bool EditableWorld::containsMap(const QString &fileName) const