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.

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

Fix various memory leaks #11578

Merged
merged 3 commits into from Feb 18, 2023

Conversation

Pokechu22
Copy link
Contributor

I fixed a few memory leaks I found with valgrind. Note that I tested on Dolphin running in WSL, which is a very jank setup. There are still a few errors reported, but I'm guessing they're related to the setup instead.

New valgrind output
$ valgrind --leak-check=full --track-origins=yes --show-leak-kinds=definite,indirect,possible Binaries/dolphin-emu
==27794== Memcheck, a memory error detector
==27794== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27794== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==27794== Command: Binaries/dolphin-emu
==27794==
==27794== error calling PR_SET_PTRACER, vgdb might block
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-pokechu22'
==27807== Warning: invalid file descriptor 1024 in syscall close()
==27807== Warning: invalid file descriptor 1025 in syscall close()
==27807== Warning: invalid file descriptor 1026 in syscall close()
==27807== Warning: invalid file descriptor 1027 in syscall close()
==27807==    Use --log-fd=<number> to select an alternative log fd.
==27807== Warning: invalid file descriptor 1028 in syscall close()
==27807== Warning: invalid file descriptor 1029 in syscall close()
==27794== Syscall param writev(vector[...]) points to uninitialised byte(s)
==27794==    at 0xA8E34BD: __writev (writev.c:26)
==27794==    by 0xA8E34BD: writev (writev.c:24)
==27794==    by 0xAAD1EC8: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0xAAD2328: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0xAAD25B6: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0xAAD2E1F: xcb_flush (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0xE234857: ??? (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0x534AC07: QWindowPrivate::applyCursor() (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x534E7CA: QWindowPrivate::setCursor(QCursor const*) (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x49F320A: ??? (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.12.8)
==27794==    by 0x49FD05F: QWidget::setCursor(QCursor const&) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.12.8)
==27794==    by 0x47D393: RenderWidget::HandleCursorTimer() (RenderWidget.cpp:191)
==27794==    by 0x5ABC327: QMetaObject::activate(QObject*, int, int, void**) (in /usr/lib/x86_64-linux-gnu/libQt5Core.so.5.12.8)
==27794==  Address 0xdcab72e is 4,590 bytes inside a block of size 21,168 alloc'd
==27794==    at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27794==    by 0xAAD1884: xcb_connect_to_fd (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0xAAD5A2D: xcb_connect_to_display_with_auth_info (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==27794==    by 0x511DD29: _XConnectXCB (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==27794==    by 0x510E3A8: XOpenDisplay (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==27794==    by 0xE23EBFF: QXcbBasicConnection::QXcbBasicConnection(char const*) (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0xE210321: QXcbConnection::QXcbConnection(QXcbNativeInterface*, bool, unsigned int, char const*) (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0xE215069: QXcbIntegration::QXcbIntegration(QStringList const&, int&, char**) (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0xE1AF512: ??? (in /usr/lib/x86_64-linux-gnu/qt5/plugins/platforms/libqxcb.so)
==27794==    by 0x532CFC2: QPlatformIntegrationFactory::create(QString const&, QStringList const&, int&, char**, QString const&) (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x533AFA0: QGuiApplicationPrivate::createPlatformIntegration() (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x533C707: QGuiApplicationPrivate::createEventDispatcher() (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==  Uninitialised value was created by a heap allocation
==27794==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27794==    by 0xE46D4E0: xcb_image_create (in /usr/lib/x86_64-linux-gnu/libxcb-image.so.0.0.0)
==27794==    by 0xE46E40E: xcb_image_native (in /usr/lib/x86_64-linux-gnu/libxcb-image.so.0.0.0)
==27794==    by 0xE46E655: xcb_create_pixmap_from_bitmap_data (in /usr/lib/x86_64-linux-gnu/libxcb-image.so.0.0.0)
==27794==    by 0xE233F5F: ??? (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0xE234312: ??? (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0xE23492A: ??? (in /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5.12.8)
==27794==    by 0x534AC07: QWindowPrivate::applyCursor() (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x534E7CA: QWindowPrivate::setCursor(QCursor const*) (in /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5.12.8)
==27794==    by 0x49F320A: ??? (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.12.8)
==27794==    by 0x49FD05F: QWidget::setCursor(QCursor const&) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.12.8)
==27794==    by 0x47D393: RenderWidget::HandleCursorTimer() (RenderWidget.cpp:191)
==27794==
==27794==
==27794== HEAP SUMMARY:
==27794==     in use at exit: 628,810 bytes in 10,187 blocks
==27794==   total heap usage: 1,979,345 allocs, 1,969,158 frees, 945,324,785 bytes allocated
==27794==
==27794== 32 bytes in 1 blocks are indirectly lost in loss record 777 of 4,062
==27794==    at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27794==    by 0xE37A93C: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE37BFDC: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE38306C: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE761A59: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE76272F: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE75FB85: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE7610CD: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE764E7F: XML_ParseBuffer (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE380F42: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE38137B: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE38158C: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==
==27794== 64 bytes in 1 blocks are definitely lost in loss record 3,605 of 4,062
==27794==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27794==    by 0x118380CD: ???
==27794==    by 0x10FC9063: ???
==27794==    by 0x4011B99: call_init.part.0 (dl-init.c:72)
==27794==    by 0x4011CA0: call_init (dl-init.c:30)
==27794==    by 0x4011CA0: _dl_init (dl-init.c:119)
==27794==    by 0xA92F984: _dl_catch_exception (dl-error-skeleton.c:182)
==27794==    by 0x401643C: dl_open_worker (dl-open.c:758)
==27794==    by 0xA92F927: _dl_catch_exception (dl-error-skeleton.c:208)
==27794==    by 0x4015609: _dl_open (dl-open.c:837)
==27794==    by 0x5DC934B: dlopen_doit (dlopen.c:66)
==27794==    by 0xA92F927: _dl_catch_exception (dl-error-skeleton.c:208)
==27794==    by 0xA92F9F2: _dl_catch_error (dl-error-skeleton.c:227)
==27794==
==27794== 288 (256 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 3,937 of 4,062
==27794==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27794==    by 0xE37A2F4: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE37A9B8: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE37BFDC: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE38306C: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE761A59: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE76272F: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE75FB85: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE7610CD: ??? (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE764E7F: XML_ParseBuffer (in /usr/lib/x86_64-linux-gnu/libexpat.so.1.6.11)
==27794==    by 0xE380F42: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==    by 0xE38137B: ??? (in /usr/lib/x86_64-linux-gnu/libfontconfig.so.1.12.0)
==27794==
==27794== LEAK SUMMARY:
==27794==    definitely lost: 320 bytes in 2 blocks
==27794==    indirectly lost: 32 bytes in 1 blocks
==27794==      possibly lost: 0 bytes in 0 blocks
==27794==    still reachable: 628,458 bytes in 10,184 blocks
==27794==         suppressed: 0 bytes in 0 blocks
==27794== Reachable blocks (those to which a pointer was found) are not shown.
==27794== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==27794==
==27794== For lists of detected and suppressed errors, rerun with: -s
==27794== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

(Those are the errors with both audio and graphics backends set to none; additional errors appear from external code when using other values, but I don't have audio actually working in WSL and the video part is a self-compiled very jank thing. Someone with an actual linux setup may want to do further testing.)

bp_item/row_item/item were never deleted, and the normal Qt ownership system wasn't applying to them because they were being cloned.
Per https://doc.qt.io/qt-6/qabstractitemview.html#setItemDelegateForColumn setItemDelegateForColumn does not take ownership of the parameter, so it was not being deleted. Specifying a parent to QObject (via QStyledItemDelegate's constructor) will allow it to automatically be deleted, per https://doc.qt.io/qt-6/objecttrees.html. The other instance of a QItemDelegate in IOWindow.cpp already used this.
We allocate in MakeSuppressor via `return unique_ptr(std::make_unique<...>(...).release(), InvokingDeleter{}`, so it wasn't properly getting freed.
@AdmiralCurtiss AdmiralCurtiss merged commit 8db35e6 into dolphin-emu:master Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants