[vlc-devel] [PATCH 02/14] qt: cancel preparse requests before destroying/changing model of NetworkMediaModel

[Thomas Guillem]

On Thu, Feb 6, 2020, at 13:56, Pierre Lamot wrote:
>   this caused a crash if the UI was destroyed when a request was pending
> ---
>  modules/gui/qt/network/networkmediamodel.cpp | 23 +++++++++++++++++++-
>  modules/gui/qt/network/networkmediamodel.hpp |  5 ++++-
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/gui/qt/network/networkmediamodel.cpp 
> b/modules/gui/qt/network/networkmediamodel.cpp
> index 1a24f3de91..dca7534438 100644
> --- a/modules/gui/qt/network/networkmediamodel.cpp
> +++ b/modules/gui/qt/network/networkmediamodel.cpp
> @@ -40,10 +40,22 @@ enum Role {
>  
>  NetworkMediaModel::NetworkMediaModel( QObject* parent )
>      : QAbstractListModel( parent )
> +    , m_preparseSem(1)
>      , m_ml( nullptr )
>  {
>  }
>  
> +NetworkMediaModel::~NetworkMediaModel()
> +{
> +    if (!m_preparseSem.tryAcquire())
> +    {
> +        auto libvlc = vlc_object_instance(m_ctx->getIntf());
> +        vlc_media_tree_PreparseCancel( libvlc, this );
> +        //wait for the callback call on cancel
> +        m_preparseSem.acquire();
> +    }
> +}
> +
>  QVariant NetworkMediaModel::data( const QModelIndex& index, int role ) const
>  {
>      if (!m_ctx)
> @@ -255,6 +267,11 @@ bool NetworkMediaModel::initializeMediaSources()
>          emit isIndexedChanged();
>      }
>  
> +    if (!m_preparseSem.tryAcquire())
> +    {
> +        vlc_media_tree_PreparseCancel( libvlc, this );
> +        m_preparseSem.acquire();
> +    }

What is the the goal of this semaphore ?

I'm afraid that using tryAcquire() is racy. 2 threads could succeed in tryAcquire(), call vlc_media_tree_PreparseCancel() and m_preparseSem.acquire() at the same time.

Anyway, I don't think it is needed. vlc_media_tree_PreparseCancel(id) will cancel every tasks that have this id. You could call it more than one time with the same id, the second time won't cancel any other tasks and do nothing.

>      vlc_media_tree_Preparse( tree, libvlc, m_treeItem.media.get(), 
> this );
>      m_parsingPending = true;
>      emit parsingPendingChanged(m_parsingPending);
> @@ -335,8 +352,12 @@ void 
> NetworkMediaModel::onItemRemoved(MediaSourcePtr, input_item_node_t * 
> node,
>      }, Qt::QueuedConnection);
>  }
>  
> -void NetworkMediaModel::onItemPreparseEnded(MediaSourcePtr, 
> input_item_node_t* node, enum input_item_preparse_status)
> +void NetworkMediaModel::onItemPreparseEnded(MediaSourcePtr, 
> input_item_node_t* node, enum input_item_preparse_status status)
>  {
> +    m_preparseSem.release();
> +    if (status != ITEM_PREPARSE_DONE)
> +        return;
> +
>      InputItemPtr p_node { node->p_item };
>      QMetaObject::invokeMethod(this, [this, p_node=std::move(p_node)]() 
> {
>          if (p_node != m_treeItem.media)
> diff --git a/modules/gui/qt/network/networkmediamodel.hpp 
> b/modules/gui/qt/network/networkmediamodel.hpp
> index bc6c2ea8b5..969d085dc4 100644
> --- a/modules/gui/qt/network/networkmediamodel.hpp
> +++ b/modules/gui/qt/network/networkmediamodel.hpp
> @@ -33,6 +33,8 @@
>  #include <util/qml_main_context.hpp>
>  #include "networksourcelistener.hpp"
>  
> +#include <QSemaphore>
> +
>  #include <memory>
>  
>  using MediaSourcePtr = vlc_shared_data_ptr_type(vlc_media_source_t,
> @@ -98,6 +100,7 @@ public:
>  
>      explicit NetworkMediaModel(QObject* parent = nullptr);
>      NetworkMediaModel( QmlMainContext* ctx, QString parentMrl, 
> QObject* parent = nullptr );
> +    virtual ~NetworkMediaModel() override;
>  
>      QVariant data(const QModelIndex& index, int role) const override;
>      QHash<int, QByteArray> roleNames() const override;
> @@ -171,7 +174,7 @@ private:
>      bool m_indexed = false;
>      bool m_canBeIndexed  = false;
>      bool m_parsingPending = false;
> -
> +    QSemaphore m_preparseSem;
>  
>      std::vector<Item> m_items;
>      QmlMainContext* m_ctx = nullptr;
> -- 
> 2.17.1
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel