Hugo Beauzée-Luyssen pushed to branch master at VideoLAN / libvlcpp
Commits:
62abf003 by Hugo Beauzée-Luyssen at 2019-06-20T16:23:25Z
EventManager: Fix potential use-after-free
When move-assigning, or copy-assigning, any object that holds an event
manager & inherits from Internal will have the internal class
copy/move constructed first, causing its internal shared pointer to
be overriden, and in turn the libvlc object to be released.
This is a problem if this object contains an event manager with
registered events, as the event manager will be overriden afterward,
causing its destructor to unregister the events after the libvlc
object it needs to unregister the events has been released.
This commit ensures we hold on to the object associated with the event
manager, and that we explicitely unregister all events before releasing
the object.
- - - - -
7 changed files:
- test/main.cpp
- vlcpp/EventManager.hpp
- vlcpp/Media.hpp
- vlcpp/MediaList.hpp
- vlcpp/MediaListPlayer.hpp
- vlcpp/MediaPlayer.hpp
- vlcpp/RendererDiscoverer.hpp
Changes:
=====================================
test/main.cpp
=====================================
@@ -157,4 +157,6 @@ int main(int ac, char** av)
std::cout << f.name() << std::endl;
}
free(imgBuffer);
+ // Check that we don't use the old media player when releasing its event
manager
+ mp = VLC::MediaPlayer{};
}
=====================================
vlcpp/EventManager.hpp
=====================================
@@ -243,8 +243,25 @@ protected:
*/
class MediaEventManager : public EventManager
{
+ private:
+ // Hold on to the object firing events. Otherwise, when copying/move
+ // assigning over its object, the Internal parent class would be
copied/moved
+ // first, and only then would the event manager be copied/moved.
+ // This can cause the last instance of the object to be released, and
+ // then used by the event manager as it unregisters its callbacks.
+ Media m_media;
+
public:
- MediaEventManager(InternalPtr ptr) : EventManager( ptr ) {}
+ MediaEventManager(InternalPtr ptr, Media m)
+ : EventManager( ptr )
+ , m_media( std::move( m ) )
+ {
+ }
+ ~MediaEventManager()
+ {
+ // Clear the events as long as the underlying VLC object is alive
+ m_lambdas.clear();
+ }
/**
* @brief onMetaChanged Registers an event called when a Media meta
changes
@@ -406,8 +423,18 @@ class MediaEventManager : public EventManager
*/
class MediaPlayerEventManager : public EventManager
{
+ private:
+ MediaPlayer m_mp;
public:
- MediaPlayerEventManager(InternalPtr ptr) : EventManager( ptr ) {}
+ MediaPlayerEventManager(InternalPtr ptr, MediaPlayer mp)
+ : EventManager( ptr )
+ , m_mp( std::move( mp ) )
+ {
+ }
+ ~MediaPlayerEventManager()
+ {
+ m_lambdas.clear();
+ }
/**
* \brief onMediaChanged Registers an event called when the played
media changes
@@ -820,8 +847,18 @@ class MediaPlayerEventManager : public EventManager
*/
class MediaListEventManager : public EventManager
{
+ private:
+ MediaList m_ml;
public:
- MediaListEventManager(InternalPtr ptr) : EventManager( ptr ) {}
+ MediaListEventManager(InternalPtr ptr, MediaList ml)
+ : EventManager( ptr )
+ , m_ml( std::move( ml ) )
+ {
+ }
+ ~MediaListEventManager()
+ {
+ m_lambdas.clear();
+ }
/**
* \brief onItemAdded Registers an event called when an item gets
added to the media list
@@ -914,8 +951,18 @@ class MediaListEventManager : public EventManager
*/
class MediaListPlayerEventManager : public EventManager
{
+ private:
+ MediaListPlayer m_mlp;
public:
- MediaListPlayerEventManager(InternalPtr ptr) : EventManager( ptr ) {}
+ MediaListPlayerEventManager(InternalPtr ptr, MediaListPlayer mlp)
+ : EventManager( ptr )
+ , m_mlp( std::move( mlp ) )
+ {
+ }
+ ~MediaListPlayerEventManager()
+ {
+ m_lambdas.clear();
+ }
template <typename Func>
RegisteredEvent onPlayed(Func&& f)
@@ -981,8 +1028,18 @@ class MediaDiscovererEventManager : public EventManager
*/
class RendererDiscovererEventManager : public EventManager
{
+private:
+ RendererDiscoverer m_rd;
public:
- RendererDiscovererEventManager( InternalPtr ptr ) : EventManager(ptr) {}
+ RendererDiscovererEventManager( InternalPtr ptr, RendererDiscoverer rd )
+ : EventManager(ptr)
+ , m_rd( std::move( rd ) )
+ {
+ }
+ ~RendererDiscovererEventManager()
+ {
+ m_lambdas.clear();
+ }
template <typename Func>
RegisteredEvent onItemAdded( Func&& f )
=====================================
vlcpp/Media.hpp
=====================================
@@ -508,7 +508,7 @@ public:
if ( m_eventManager == nullptr )
{
libvlc_event_manager_t* obj = libvlc_media_event_manager(*this);
- m_eventManager = std::make_shared<MediaEventManager>( obj );
+ m_eventManager = std::make_shared<MediaEventManager>( obj, *this );
}
return *m_eventManager;
}
=====================================
vlcpp/MediaList.hpp
=====================================
@@ -236,7 +236,7 @@ public:
if ( m_eventManager == nullptr )
{
libvlc_event_manager_t* obj = libvlc_media_list_event_manager(
*this );
- m_eventManager = std::make_shared<MediaListEventManager>( obj );
+ m_eventManager = std::make_shared<MediaListEventManager>( obj,
*this );
}
return *m_eventManager;
}
=====================================
vlcpp/MediaListPlayer.hpp
=====================================
@@ -76,7 +76,7 @@ public:
if ( m_eventManager == nullptr )
{
libvlc_event_manager_t* obj =
libvlc_media_list_player_event_manager(*this);
- m_eventManager = std::make_shared<MediaListPlayerEventManager>(
obj );
+ m_eventManager = std::make_shared<MediaListPlayerEventManager>(
obj, *this );
}
return *m_eventManager;
}
=====================================
vlcpp/MediaPlayer.hpp
=====================================
@@ -149,7 +149,7 @@ public:
if ( m_eventManager == nullptr )
{
libvlc_event_manager_t* obj = libvlc_media_player_event_manager(
*this );
- m_eventManager = std::make_shared<MediaPlayerEventManager>( obj );
+ m_eventManager = std::make_shared<MediaPlayerEventManager>( obj,
*this );
}
return *m_eventManager;
}
=====================================
vlcpp/RendererDiscoverer.hpp
=====================================
@@ -95,7 +95,7 @@ public:
if ( m_eventManager == nullptr )
{
libvlc_event_manager_t* obj =
libvlc_renderer_discoverer_event_manager( *this );
- m_eventManager = std::make_shared<RendererDiscovererEventManager>(
obj );
+ m_eventManager = std::make_shared<RendererDiscovererEventManager>(
obj, *this );
}
return *m_eventManager;
}
View it on GitLab:
https://code.videolan.org/videolan/libvlcpp/commit/62abf00340cd49ad54075bb4a9b8b24471a0c22b
--
View it on GitLab:
https://code.videolan.org/videolan/libvlcpp/commit/62abf00340cd49ad54075bb4a9b8b24471a0c22b
You're receiving this email because of your account on code.videolan.org.
_______________________________________________
vlc-commits mailing list
[email protected]
https://mailman.videolan.org/listinfo/vlc-commits
