Commit 62abf003 authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen

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.
parent dcdf3470
......@@ -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{};
}
......@@ -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 )
......
......@@ -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;
}
......
......@@ -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;
}
......
......@@ -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;
}
......
......@@ -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;
}
......
......@@ -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;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment