[vlc-commits] [Git][videolan/vlc][master] 3 commits: qt: compositor: fix leak on compositor failure

[Rémi Denis-Courmont (@Courmisch)]

Rémi Denis-Courmont pushed to branch master at VideoLAN / VLC


Commits:
da87c329 by Alexandre Janniaux at 2024-01-06T09:08:10+00:00
qt: compositor: fix leak on compositor failure

When a compositor was allocated but its initialization failed, the
pointer would not be returned by the
vlc::CompositorFactory::createCompositor function and the pointer was
leaked.

Fix the following leak:

    Indirect leak of 120 byte(s) in 1 object(s) allocated from:
        #0 0x7f806a2e2002 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
        #1 0x7f80550b07b3 in instanciateCompositor<vlc::CompositorX11> ../../../../modules/gui/qt/maininterface/compositor.cpp:45
        #2 0x7f80550b59e8 in vlc::CompositorFactory::createCompositor() ../../../../modules/gui/qt/maininterface/compositor.cpp:95
        #3 0x7f80546daafa in Thread ../../../../modules/gui/qt/qt.cpp:822
        #4 0x7f80688aa9ea  (/usr/lib/libc.so.6+0x8c9ea) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)

- - - - -
494ced7b by Alexandre Janniaux at 2024-01-06T09:08:10+00:00
qt: use unique_ptr for compositor

The code is already defending against nullptr since multiple clients can
destroy the pointer, and the compositor might not be allocated in the
situation where Qt is allocated as a dialog provider, so there is no
reason to not use std::unique_ptr here.

Previous commits also ensure that vlc_object_create<>() will correctly
initialize C++ objects, and vlc_object_destroy<>() will correctly
destroy them.

No functional changes.

- - - - -
4df5dbc2 by Alexandre Janniaux at 2024-01-06T09:08:10+00:00
qt: systempalette: use C++ wrapper for vlc_object_create

The C++ wrapper is simpler to write and would initialize C++ objects
correctly, though they are not required here.

- - - - -


4 changed files:

- modules/gui/qt/maininterface/compositor.cpp
- modules/gui/qt/qt.cpp
- modules/gui/qt/qt.hpp
- modules/gui/qt/style/systempalette.cpp


Changes:

=====================================
modules/gui/qt/maininterface/compositor.cpp
=====================================
@@ -92,12 +92,15 @@ Compositor* CompositorFactory::createCompositor()
     {
         if (m_compositorName == "auto" || m_compositorName == compositorList[m_compositorIndex].name)
         {
-            Compositor* compositor = compositorList[m_compositorIndex].instantiate(m_intf);
+            std::unique_ptr<Compositor> compositor {
+                compositorList[m_compositorIndex].instantiate(m_intf)
+            };
+
             if (compositor->init())
             {
                 //avoid looping over the same compositor if the current ones fails further initialisation steps
                 m_compositorIndex++;
-                return compositor;
+                return compositor.release();
             }
         }
     }


=====================================
modules/gui/qt/qt.cpp
=====================================
@@ -596,6 +596,8 @@ static int OpenIntfCommon( vlc_object_t *p_this, bool dialogProvider )
 {
     auto intfThread = reinterpret_cast<intf_thread_t*>(p_this);
     libvlc_int_t *libvlc = vlc_object_instance( p_this );
+
+    /* Ensure initialization of objects in qt_intf_t. */
     auto p_intf = vlc_object_create<qt_intf_t>( libvlc );
     if (!p_intf)
         return VLC_ENOMEM;
@@ -609,6 +611,7 @@ static int OpenIntfCommon( vlc_object_t *p_this, bool dialogProvider )
     p_intf->b_isDialogProvider = dialogProvider;
     p_intf->isShuttingDown = false;
     p_intf->refCount = 1;
+
     int ret = OpenInternal(p_intf);
     if (ret != VLC_SUCCESS)
     {
@@ -819,15 +822,14 @@ static void *Thread( void *obj )
     {
         bool ret = false;
         do {
-            p_intf->p_compositor = compositorFactory.createCompositor();
+            p_intf->p_compositor.reset(compositorFactory.createCompositor());
             if (! p_intf->p_compositor)
                 break;
             ret = p_intf->p_compositor->makeMainInterface(p_intf->p_mi);
             if (!ret)
             {
                 p_intf->p_compositor->destroyMainInterface();
-                delete p_intf->p_compositor;
-                p_intf->p_compositor = nullptr;
+                p_intf->p_compositor.reset();
             }
         } while(!ret);
 
@@ -939,8 +941,7 @@ static void *ThreadCleanup( qt_intf_t *p_intf, CleanupReason cleanupReason )
             delete p_intf->mainSettings;
             p_intf->mainSettings = nullptr;
 
-            delete p_intf->p_compositor;
-            p_intf->p_compositor = nullptr;
+            p_intf->p_compositor.reset();
         }
     }
 


=====================================
modules/gui/qt/qt.hpp
=====================================
@@ -32,6 +32,8 @@
 #include <vlc_configuration.h>
 #include <vlc_threads.h>
 
+#include <memory>
+
 #include <qconfig.h>
 
 #define QT_NO_CAST_TO_ASCII
@@ -106,7 +108,7 @@ struct qt_intf_t
     vlc_player_t *p_player; /* player */
     vlc::playlist::PlaylistController* p_mainPlaylistController;
     PlayerController* p_mainPlayerController;
-    vlc::Compositor*  p_compositor;
+    std::unique_ptr<vlc::Compositor>  p_compositor;
 
 #ifdef _WIN32
     bool disable_volume_keys;


=====================================
modules/gui/qt/style/systempalette.cpp
=====================================
@@ -185,7 +185,7 @@ bool ExternalPaletteImpl::init()
         preferedProvider = "qt-themeprovider-gtk";
 #endif
 
-    m_provider = static_cast<vlc_qt_theme_provider_t*>(vlc_object_create(m_ctx->getIntf(), sizeof(vlc_qt_theme_provider_t)));
+    m_provider = vlc_object_create<vlc_qt_theme_provider_t>(m_ctx->getIntf());
     if (!m_provider)
         return false;
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9b4a4753ececcb8e98349f81b3a303370061cd1f...4df5dbc2458f96fbb7edce7d9e55ca8955bbd048

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9b4a4753ececcb8e98349f81b3a303370061cd1f...4df5dbc2458f96fbb7edce7d9e55ca8955bbd048
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance