[vlc-devel] [PATCH 3/5] medialib: fix initialization

Romain Vimont rom1v at videolabs.io
Thu Jan 7 13:23:59 UTC 2021


The medialib initialization should be called exactly once.

This was implemented by two mechanisms. Firstly, m_ml->isInitialized()
was checked to avoid any work if the initialization already occurred.
But since the medialib mutex was hold only during the call to
isInitialized(), as soon as it returns, another thread may have set
m_initialized. To handle this case harmlessly, Init() checks if
m_ml->initialize() returns AlreadyInitialized.

However, between the call to isInitialized() and initialize(), several
medialibrary methods must be called (registerDeviceLister() and
addFileSystemFactory()). These methods assert that m_initialized is
false, which was not guaranteed.

To fix this race condition, use our own flag protected by a local mutex,
hold during the whole Init() execution.

This also avoids a lot of unnecessary blocking calls: many medialibrary
controls (including the asynchronous ones) need to call Init() just to
make sure that the medialib was initialized. But since isInitialized()
is protected by the medialibray mutex, all these calls had to wait for
the mutex (hold during the execution of SQL queries) to be released.
With a separate mutex, this problem disappears.
---
 modules/misc/medialibrary/medialibrary.cpp | 6 +++++-
 modules/misc/medialibrary/medialibrary.h   | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/modules/misc/medialibrary/medialibrary.cpp b/modules/misc/medialibrary/medialibrary.cpp
index a58032668d..ff675cf865 100644
--- a/modules/misc/medialibrary/medialibrary.cpp
+++ b/modules/misc/medialibrary/medialibrary.cpp
@@ -388,8 +388,10 @@ MediaLibrary::MediaLibrary( vlc_medialibrary_module_t* ml )
 
 bool MediaLibrary::Init()
 {
-    if ( m_ml->isInitialized() == true )
+    vlc::threads::mutex_locker lock( m_mutex );
+    if( m_initialized )
         return true;
+
     auto userDir = vlc::wrap_cptr( config_GetUserDir( VLC_USERDATA_DIR ) );
     std::string mlDir = std::string{ userDir.get() } + "/ml/";
 
@@ -403,6 +405,7 @@ bool MediaLibrary::Init()
     switch ( initStatus )
     {
         case medialibrary::InitializeResult::AlreadyInitialized:
+            assert(!"initialize() should not have been called if already initialized");
             return true;
         case medialibrary::InitializeResult::Failed:
             msg_Err( m_vlc_ml, "Medialibrary failed to initialize" );
@@ -450,6 +453,7 @@ bool MediaLibrary::Init()
 
     m_ml->setDiscoverNetworkEnabled( true );
 
+    m_initialized = true;
     return true;
 }
 
diff --git a/modules/misc/medialibrary/medialibrary.h b/modules/misc/medialibrary/medialibrary.h
index 5fc0e28d99..09c773bf59 100644
--- a/modules/misc/medialibrary/medialibrary.h
+++ b/modules/misc/medialibrary/medialibrary.h
@@ -167,6 +167,9 @@ private:
     std::unique_ptr<Logger> m_logger;
     std::unique_ptr<medialibrary::IMediaLibrary> m_ml;
 
+    vlc::threads::mutex m_mutex;
+    bool m_initialized = false; /* protected by m_mutex */
+
     // IMediaLibraryCb interface
 public:
     virtual void onMediaAdded(std::vector<medialibrary::MediaPtr> media) override;
-- 
2.30.0



More information about the vlc-devel mailing list