Fix thread-safety issues when initially loading devices from the database.

When DeviceManager initializes, it creates a thread to load device information
from the database. Part of this process includes use of QPixMap for icons which
produced a warning message:

22:32:53.763 WARN  unknown                          QPixmap: It is not safe to use pixmaps outside the GUI thread

In addition, the device is added to the view using beginInsertRows and
endInsertRows. This could contend with a device added by a lister signaling
PhysicalDeviceAdded.

To solve these problems, this change moves the icon loading and insertion to the
main thread. LoadAllDevices reads the data from the database and creates the
DeviceInfo object, then sends a signal to the main thread. In the signal
handler, the icon is loaded and the device is added to the master list and view.
This commit is contained in:
Jim Broadus 2019-02-23 22:12:07 -08:00
parent 248f1d8596
commit a62062127e
3 changed files with 25 additions and 12 deletions

View File

@ -58,14 +58,9 @@ void DeviceInfo::InitFromDb(const DeviceDatabaseBackend::Device& dev) {
size_ = dev.size_;
transcode_mode_ = dev.transcode_mode_;
transcode_format_ = dev.transcode_format_;
QStringList icon_names = dev.icon_name_.split(',');
QVariantList icons;
for (const QString& icon_name : icon_names) {
icons << icon_name;
}
LoadIcon(icons, friendly_name_);
// Store the raw value for now. If it's a comma delimited list, it will be
// sorted out later.
icon_name_ = dev.icon_name_;
QStringList unique_ids = dev.unique_id_.split(',');
for (const QString& id : unique_ids) {

View File

@ -83,6 +83,8 @@ DeviceManager::DeviceManager(Application* app, QObject* parent)
backend_->moveToThread(app_->database()->thread());
backend_->Init(app_->database());
connect(this, SIGNAL(DeviceCreatedFromDb(DeviceInfo*)),
SLOT(AddDeviceFromDb(DeviceInfo*)));
// This reads from the database and contends on the database mutex, which can
// be very slow on startup.
ConcurrentRun::Run<void>(&thread_pool_,
@ -140,13 +142,27 @@ void DeviceManager::LoadAllDevices() {
for (const DeviceDatabaseBackend::Device& device : devices) {
DeviceInfo* info = new DeviceInfo(DeviceInfo::Type_Device, root_);
info->InitFromDb(device);
beginInsertRows(ItemToIndex(root_), devices_.count(), devices_.count());
devices_ << info;
endInsertRows();
// Use of QPixMap and device insertion should only be done on the main
// thread. Send a signal to finish the device addition.
emit DeviceCreatedFromDb(info);
}
}
void DeviceManager::AddDeviceFromDb(DeviceInfo* info) {
// At this point, icon_name_ contains the value from the database where the
// value is allowed to be a comma delimited list.
QStringList icon_names = info->icon_name_.split(',');
QVariantList icons;
for (const QString& icon_name : icon_names) {
icons << icon_name;
}
info->LoadIcon(icons, info->friendly_name_);
beginInsertRows(ItemToIndex(root_), devices_.count(), devices_.count());
devices_ << info;
endInsertRows();
}
QVariant DeviceManager::data(const QModelIndex& idx, int role) const {
if (!idx.isValid() || idx.column() != 0) return QVariant();

View File

@ -101,6 +101,7 @@ class DeviceManager : public SimpleTreeModel<DeviceInfo> {
signals:
void DeviceConnected(QModelIndex idx);
void DeviceDisconnected(QModelIndex idx);
void DeviceCreatedFromDb(DeviceInfo* info);
private slots:
void PhysicalDeviceAdded(const QString& id);
@ -111,6 +112,7 @@ class DeviceManager : public SimpleTreeModel<DeviceInfo> {
void DeviceSongCountUpdated(int count);
void LoadAllDevices();
void DeviceConnectFinished(const QString& id, bool success);
void AddDeviceFromDb(DeviceInfo* info);
protected:
void LazyPopulate(DeviceInfo* item) { LazyPopulate(item, true); }