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.

Sign up for GitHub

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

Jump to bottom

Tiled crashing due to double-deletion of Tilesets #3290

Closed
Miepee opened this issue Mar 5, 2022 · 3 comments
Closed

Tiled crashing due to double-deletion of Tilesets #3290

Miepee opened this issue Mar 5, 2022 · 3 comments
Labels
bug Broken behavior.

Comments

@Miepee
Copy link
Contributor

Miepee commented Mar 5, 2022

Describe the bug
Closing a map with a lot of tilesets causes Tiled to crash

To Reproduce

  • Create a map
  • add a lot of tilesets to the map
  • close the map

Expected behavior
Tiled should only close the map, and not crash.

Media
Backtrace:

Thread 1 "tiled" received signal SIGSEGV, Segmentation fault.
0x00005555556be3f4 in std::default_delete<Tiled::EditableAsset>::operator() (this=0x555558887098, __ptr=0x555556eef700) at /usr/include/c++/11.2.0/bits/unique_ptr.h:85
85              delete __ptr;
(gdb) bt
#0  0x00005555556be3f4 in std::default_delete<Tiled::EditableAsset>::operator() (this=0x555558887098, __ptr=0x555556eef700) at /usr/include/c++/11.2.0/bits/unique_ptr.h:85
#1  0x00005555556be962 in std::__uniq_ptr_impl<Tiled::EditableAsset, std::default_delete<Tiled::EditableAsset> >::reset (this=0x555558887098, __p=0x0)
    at /usr/include/c++/11.2.0/bits/unique_ptr.h:182
#2  0x0000555555778c69 in std::unique_ptr<Tiled::EditableAsset, std::default_delete<Tiled::EditableAsset> >::reset (this=0x555558887098, __p=0x0)
    at /usr/include/c++/11.2.0/bits/unique_ptr.h:456
#3  0x00005555558ed9bc in Tiled::TilesetDocument::~TilesetDocument (this=0x555558887060, __in_chrg=<optimized out>)
    at /mnt/c/Users/narr/gitrepos/tiled/src/tiled/tilesetdocument.cpp:103
#4  0x00005555556d0516 in QtSharedPointer::ExternalRefCountWithContiguousData<Tiled::TilesetDocument>::deleter (self=0x555558887050)
    at /usr/include/qt/QtCore/qsharedpointer_impl.h:248
#5  0x0000555555606aef in QtSharedPointer::ExternalRefCountData::destroy (this=0x555558887050) at /usr/include/qt/QtCore/qsharedpointer_impl.h:149
#6  0x00005555556d0d1e in QSharedPointer<Tiled::TilesetDocument>::deref (dd=0x555558887050) at /usr/include/qt/QtCore/qsharedpointer_impl.h:458
#7  0x00005555556ce822 in QSharedPointer<Tiled::TilesetDocument>::deref (this=0x7fffffffd1b0) at /usr/include/qt/QtCore/qsharedpointer_impl.h:453
#8  0x00005555556cb98c in QSharedPointer<Tiled::TilesetDocument>::~QSharedPointer (this=0x7fffffffd1b0, __in_chrg=<optimized out>)
    at /usr/include/qt/QtCore/qsharedpointer_impl.h:310
#9  0x00005555556c5cdf in Tiled::DocumentManager::removeFromTilesetDocument (this=0x555555f1a610, tileset=..., mapDocument=0x555556ef3310)
    at /mnt/c/Users/narr/gitrepos/tiled/src/tiled/documentmanager.cpp:1215
#10 0x00005555556c4026 in Tiled::DocumentManager::closeDocumentAt (this=0x555555f1a610, index=0) at /mnt/c/Users/narr/gitrepos/tiled/src/tiled/documentmanager.cpp:880
#11 0x0000555555753d40 in Tiled::MainWindow::closeDocument (this=0x7fffffffe5d0, index=0) at /mnt/c/Users/narr/gitrepos/tiled/src/tiled/mainwindow.cpp:2503
#12 0x000055555576c05e in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<int>, void, void (Tiled::MainWindow::*)(int)>::call (f=
    (void (Tiled::MainWindow::*)(Tiled::MainWindow * const, int)) 0x555555753cd8 <Tiled::MainWindow::closeDocument(int)>, o=0x7fffffffe5d0, arg=0x7fffffffd440)
    at /usr/include/qt/QtCore/qobjectdefs_impl.h:152
#13 0x000055555576aaf7 in QtPrivate::FunctionPointer<void (Tiled::MainWindow::*)(int)>::call<QtPrivate::List<int>, void> (f=
    (void (Tiled::MainWindow::*)(Tiled::MainWindow * const, int)) 0x555555753cd8 <Tiled::MainWindow::closeDocument(int)>, o=0x7fffffffe5d0, arg=0x7fffffffd440)
    at /usr/include/qt/QtCore/qobjectdefs_impl.h:185
#14 0x0000555555769017 in QtPrivate::QSlotObject<void (Tiled::MainWindow::*)(int), QtPrivate::List<int>, void>::impl (which=1, this_=0x5555564cf220, r=0x7fffffffe5d0,
    a=0x7fffffffd440, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:418
#15 0x00007ffff68c09d3 in ?? () from /usr/lib/libQt5Core.so.5
#16 0x00005555556c8029 in Tiled::DocumentManager::documentCloseRequested (this=0x555555f1a610, _t1=0)
    at /mnt/c/Users/narr/gitrepos/tiled/default/tiled.1920fdf8/qt.headers/moc_documentmanager.cpp:533
#17 0x00005555556d50a3 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<int>, void, void (Tiled::DocumentManager::*)(int)>::call (f=
    (void (Tiled::DocumentManager::*)(Tiled::DocumentManager * const, int)) 0x5555556c7fcc <Tiled::DocumentManager::documentCloseRequested(int)>, o=0x555555f1a610,
    arg=0x7fffffffd610) at /usr/include/qt/QtCore/qobjectdefs_impl.h:152
#18 0x00005555556d39e7 in QtPrivate::FunctionPointer<void (Tiled::DocumentManager::*)(int)>::call<QtPrivate::List<int>, void> (f=
    (void (Tiled::DocumentManager::*)(Tiled::DocumentManager * const, int)) 0x5555556c7fcc <Tiled::DocumentManager::documentCloseRequested(int)>, o=0x555555f1a610,
    arg=0x7fffffffd610) at /usr/include/qt/QtCore/qobjectdefs_impl.h:185
#19 0x00005555556d136f in QtPrivate::QSlotObject<void (Tiled::DocumentManager::*)(int), QtPrivate::List<int>, void>::impl (which=1, this_=0x555555f0a680, r=0x555555f1a610,
    a=0x7fffffffd610, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:418
#20 0x00007ffff68c09d3 in ?? () from /usr/lib/libQt5Core.so.5
#21 0x00007ffff76198d6 in QTabBar::tabCloseRequested(int) () from /usr/lib/libQt5Widgets.so.5
#22 0x00007ffff68c0a1f in ?? () from /usr/lib/libQt5Core.so.5
#23 0x00007ffff754bdb7 in QAbstractButton::clicked(bool) () from /usr/lib/libQt5Widgets.so.5
#24 0x00007ffff754dc2c in ?? () from /usr/lib/libQt5Widgets.so.5
#25 0x00007ffff755141a in ?? () from /usr/lib/libQt5Widgets.so.5
#26 0x00007ffff75515e8 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () from /usr/lib/libQt5Widgets.so.5
#27 0x00007ffff749d616 in QWidget::event(QEvent*) () from /usr/lib/libQt5Widgets.so.5
#28 0x00007ffff74671a6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#29 0x00007ffff746bfd7 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#30 0x00007ffff688fb9a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt5Core.so.5
#31 0x00007ffff746a99f in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) ()
   from /usr/lib/libQt5Widgets.so.5
#32 0x00007ffff74bb7d7 in ?? () from /usr/lib/libQt5Widgets.so.5
#33 0x00007ffff74bd37c in ?? () from /usr/lib/libQt5Widgets.so.5
#34 0x00007ffff74671a6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#35 0x00007ffff688fb9a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt5Core.so.5
#36 0x00007ffff6c63130 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) () from /usr/lib/libQt5Gui.so.5
#37 0x00007ffff6c4e695 in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Gui.so.5
#38 0x00007ffff2b27fc0 in ?? () from /usr/lib/libQt5XcbQpa.so.5
#39 0x00007ffff5297ee3 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#40 0x00007ffff52ee0f9 in ?? () from /usr/lib/libglib-2.0.so.0
#41 0x00007ffff5295455 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#42 0x00007ffff68dbada in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#43 0x00007ffff6887e6b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#44 0x00007ffff68935c7 in QCoreApplication::exec() () from /usr/lib/libQt5Core.so.5
#45 0x000055555573e987 in main (argc=1, argv=0x7fffffffe8a8) at /mnt/c/Users/narr/gitrepos/tiled/src/tiled/main.cpp:528

Platform:

  • OS: Windows 10, Arch Linux under WSL2
  • Tiled 1.8.2 and from commit 508aa47
@bjorn
Copy link
Member

bjorn commented Mar 7, 2022

If you actually have a map with lots of tilesets, would you be able to share it for reproducing the problem?

Or, if you have a script that sets up a map with lots of tilesets, could you share that?

@bjorn bjorn added the bug Broken behavior. label Mar 7, 2022
@Miepee
Copy link
Contributor Author

Miepee commented Mar 7, 2022

Here's a script+file to reproduce:
bug3290.zip
Change line 7 to point to any local image, put the script then into the extension folder and load up the json2 file.

Something I found out while trying to prepare this script:

  • saving the loaded Map as a TMX, reopening it and closing it does not crash.
  • removing lines 38-40 (responsible for setting tiles) also doesn't cause a crash.

Which might mean the crash is actually due to having tiles Out-of-Bounds, rather than having too many tilesets.

@bjorn
Copy link
Member

bjorn commented Mar 7, 2022

So, the problem here is similar to #3292, though it's a bit of a different case.

All the tilesets you've created are EditableTileset instances. Upon opening that map in Tiled, ownership of these existing wrapper objects is moved to C++. But apparently it can be already too late, so those objects are already marked for deletion, and then the crash happens on closing the tab due to a double-deletion.

bjorn added a commit that referenced this issue Mar 7, 2022
@bjorn
Scripting: Avoid possibly crash due to garbage collection
bc7a541 
Don't return pointers of objects that are about to be deleted.

Closes #3290
@Miepee Miepee changed the title Closing a map with a lot of tilesets causes Tiled to crash Tiled crashing due to double-deletion of Tilesets Mar 7, 2022
bjorn added a commit that referenced this issue Mar 8, 2022
@bjorn
Scripting: Avoid possible crash due to garbage collection
3218d03 
Don't use editable wrapper objects that are about to be deleted. In
particular, avoid attempting to take ownership of them, which caused a
double-deletion.

Also detaching and attaching now only happens with editable wrapper
objects that aren't marked for deletion.

When an editable wrapper gets deleted, it now only removes itself from
EditableManager::mEditables, whereas it could otherwise delete an entry
for a more recently created editable wrapper.

The QJSEngine::newQObject(object).isNull() check was replaced by the
private API QQmlData::wasDeleted, since newQObject could crash when
being called during script engine destruction.

Closes #3290
bjorn added a commit that referenced this issue Mar 8, 2022
@bjorn
Scripting: Avoid possible crash due to garbage collection
5403e2d 
Don't use editable wrapper objects that are about to be deleted. In
particular, avoid attempting to take ownership of them, which caused a
double-deletion.

Also detaching and attaching now only happens with editable wrapper
objects that aren't marked for deletion.

When an editable wrapper gets deleted, it now only removes itself from
EditableManager::mEditables, whereas it could otherwise delete an entry
for a more recently created editable wrapper.

The QJSEngine::newQObject(object).isNull() check was replaced by the
private API QQmlData::wasDeleted, since newQObject could crash when
being called during script engine destruction.

Closes #3290
@bjorn bjorn mentioned this issue Mar 8, 2022
Closed
@bjorn bjorn closed this as completed in 9bad4fa Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Broken behavior.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bjorn @Miepee