From e1c719784a7fabf3d3e8cd6a250f8bfb3f4d2bad Mon Sep 17 00:00:00 2001
From: Fatih Uzunoglu <fuzun54@outlook.com>
Date: Sat, 29 Mar 2025 21:09:26 +0200
Subject: [PATCH 1/2] qt: use `QPointer` for `QObject` members of `qt_intf_t`
Although the pointers were set to null after the objects
are destroyed, if for another reason the objects were
destroyed (or never constructed and the pointers were
not initialized), using a weak pointer can prevent
unintentionally dereferencing dangling pointers.
---
modules/gui/qt/qt.cpp | 19 +++----------------
modules/gui/qt/qt.hpp | 13 +++++++------
modules/gui/qt/util/model_recovery_agent.hpp | 2 +-
3 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
index 57d25ed5dc14..aeabf0beb988 100644
--- a/modules/gui/qt/qt.cpp
+++ b/modules/gui/qt/qt.cpp
@@ -1087,7 +1087,6 @@ static void *Thread( void *obj )
{
msg_Err(p_intf, "unable to create main interface");
delete p_intf->p_mi;
- p_intf->p_mi = nullptr;
//process deleteLater events as the main loop will never run
QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
return ThreadCleanup( p_intf, CLEANUP_ERROR );
@@ -1180,32 +1179,20 @@ static void *ThreadCleanup( qt_intf_t *p_intf, CleanupReason cleanupReason )
MediaSourceCache::killInstance();
//destroy MainCtx, Compositor shouldn't not use MainCtx after `unloadGUI`
- if (p_intf->p_mi) {
- delete p_intf->p_mi;
- p_intf->p_mi = nullptr;
- }
+ delete p_intf->p_mi;
if ( p_intf->p_compositor && cleanupReason == CLEANUP_APP_TERMINATED)
{
p_intf->p_compositor.reset();
delete p_intf->mainSettings;
- p_intf->mainSettings = nullptr;
}
/* Destroy the main playlist controller */
- if (p_intf->p_mainPlaylistController)
- {
- delete p_intf->p_mainPlaylistController;
- p_intf->p_mainPlaylistController = nullptr;
- }
+ delete p_intf->p_mainPlaylistController;
/* Destroy the main InputManager */
- if (p_intf->p_mainPlayerController)
- {
- delete p_intf->p_mainPlayerController;
- p_intf->p_mainPlayerController = nullptr;
- }
+ delete p_intf->p_mainPlayerController;
/* Delete the application automatically */
return NULL;
diff --git a/modules/gui/qt/qt.hpp b/modules/gui/qt/qt.hpp
index b2a400e91e35..373522df7d45 100644
--- a/modules/gui/qt/qt.hpp
+++ b/modules/gui/qt/qt.hpp
@@ -37,6 +37,7 @@
#include <qconfig.h>
#include <QString>
+#include <QPointer>
enum {
IMEventTypeOffset = 0,
@@ -83,15 +84,15 @@ struct qt_intf_t
vlc_thread_t thread;
- class MainCtx *p_mi; /* Main Interface, NULL if DialogProvider Mode */
- class QSettings *mainSettings; /* Qt State settings not messing main VLC ones */
+ QPointer<class MainCtx> p_mi; /* Main Interface, NULL if DialogProvider Mode */
+ QPointer<class QSettings> mainSettings; /* Qt State settings not messing main VLC ones */
bool b_isDialogProvider; /* Qt mode or Skins mode */
vlc_playlist_t *p_playlist; /* playlist */
vlc_player_t *p_player; /* player */
- vlc::playlist::PlaylistController* p_mainPlaylistController;
- PlayerController* p_mainPlayerController;
+ QPointer<vlc::playlist::PlaylistController> p_mainPlaylistController;
+ QPointer<PlayerController> p_mainPlayerController;
std::unique_ptr<vlc::Compositor> p_compositor;
int refCount;
@@ -116,8 +117,8 @@ public:
};
#define THEDP DialogsProvider::getInstance()
-#define THEMIM p_intf->p_mainPlayerController
-#define THEMPL p_intf->p_mainPlaylistController
+#define THEMIM p_intf->p_mainPlayerController.get()
+#define THEMPL p_intf->p_mainPlaylistController.get()
#define qfu( i ) QString::fromUtf8( i )
#define qfue( i ) QString::fromUtf8( i ).replace( "&", "&&" ) /* for actions/buttons */
diff --git a/modules/gui/qt/util/model_recovery_agent.hpp b/modules/gui/qt/util/model_recovery_agent.hpp
index 3e2f36462e4d..5435bd50eb9d 100644
--- a/modules/gui/qt/util/model_recovery_agent.hpp
+++ b/modules/gui/qt/util/model_recovery_agent.hpp
@@ -45,7 +45,7 @@ class ModelRecoveryAgent
public:
// NOTE: settings and model must outlive the instance of this class.
template<class T>
- ModelRecoveryAgent(class QSettings *settings, const QString& modelIdentifier, T* model)
+ ModelRecoveryAgent(QPointer<QSettings> settings, const QString& modelIdentifier, QPointer<T> model)
: m_settings(settings), m_key(modelIdentifier + QStringLiteral("/RecoveryFilePath"))
{
assert(settings);
--
GitLab
From 354d167ba0561ebace42751dae09963fb39da17c Mon Sep 17 00:00:00 2001
From: Fatih Uzunoglu <fuzun54@outlook.com>
Date: Sat, 29 Mar 2025 21:17:30 +0200
Subject: [PATCH 2/2] qt: do not construct `ModelRecoveryAgent` if settings or
playlist controller do not exist
`ModelRecoveryAgent` requires valid settings and playlist controller instances. It can
not be constructed when they do not exist. This is an extremely unlikely case (but not
impossible), because given the structure of the application it is expected that they
are valid when `ModelRecoveryAgent` is constructed.
It should be noted that the reason `ModelRecoveryAgent` construction is delayed through
queued connection is only to not prolong the application startup time.
---
modules/gui/qt/qt.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
index aeabf0beb988..3265a2637c51 100644
--- a/modules/gui/qt/qt.cpp
+++ b/modules/gui/qt/qt.cpp
@@ -1027,11 +1027,12 @@ static void *Thread( void *obj )
p_intf->p_mainPlaylistController = new vlc::playlist::PlaylistController(p_intf->p_playlist);
std::unique_ptr<ModelRecoveryAgent> playlistModelRecoveryAgent;
- QMetaObject::invokeMethod(&app, [&playlistModelRecoveryAgent, p_intf]() {
+ QMetaObject::invokeMethod(&app, [&playlistModelRecoveryAgent, settings = p_intf->mainSettings, playlistController = p_intf->p_mainPlaylistController]() {
try {
- playlistModelRecoveryAgent = std::make_unique<ModelRecoveryAgent>(p_intf->mainSettings,
- QStringLiteral("Playlist"),
- p_intf->p_mainPlaylistController);
+ if (Q_LIKELY(settings && playlistController))
+ playlistModelRecoveryAgent = std::make_unique<ModelRecoveryAgent>(settings,
+ QStringLiteral("Playlist"),
+ playlistController);
} catch (...){ }
}, Qt::QueuedConnection);
--
GitLab