Commit 3076baff authored by David Jarvie's avatar David Jarvie
Browse files

Fix crash when a resource is removed

This may only happen using development builds.
parent 681e0892
Pipeline #111312 passed with stage
in 1 minute and 26 seconds
KAlarm Change Log
=== Version 3.3.4 (KDE Applications 21.12.1) --- 16 December 2021 ===
=== Version 3.3.4 (KDE Applications 21.12.1) --- 17 December 2021 ===
* Treat empty read-only, or non-existent, calendar files as loaded.
* Ensure KAlarm command line actions are performed if KAlarm is already running [KDE Bug 446749]
* Don't disable alarms after KAlarm command line action while KAlarm is already running [KDE Bug 446749]
* Fix resource ID numbers not working in command line & DBUS commands.
* If KAlarm is already running, don't exit if a new activation has unknown command line options.
* Fix crash when a resource is removed.
=== Version 3.3.3 (KDE Applications 21.12) --- 7 November 2021 ===
* Show numbers in localised form.
......
......@@ -33,7 +33,7 @@ FileResource::~FileResource()
bool FileResource::isValid() const
{
#ifdef DEBUG_DETAIL
if (!mSettings->isValid())
if (!mSettings || !mSettings->isValid())
qDebug() << "FileResource::isValid:" << displayId() << "NO (mSettings)";
if (mStatus == Status::NotConfigured)
qDebug() << "FileResource::isValid:" << displayId() << "NO (Not configured)";
......@@ -41,11 +41,11 @@ bool FileResource::isValid() const
qDebug() << "FileResource::isValid:" << displayId() << "NO (Closed)";
if (id() < 0)
qDebug() << "FileResource::isValid:" << displayId() << "NO (ID < 0)";
if (mSettings->id() != id())
if (mSettings && mSettings->id() != id())
qDebug() << "FileResource::isValid:" << displayId() << "NO (ID:" << id() << mSettings->id() << ")";
#endif
// The settings ID must not have changed since construction.
return mSettings->isValid()
return mSettings && mSettings->isValid()
&& mStatus < Status::Unusable
&& id() >= 0 && mSettings->id() == id();
}
......@@ -57,6 +57,8 @@ ResourceId FileResource::displayId() const
QString FileResource::storageTypeString(bool description) const
{
if (!mSettings)
return {};
bool file;
switch (mSettings->storageType())
{
......@@ -74,67 +76,76 @@ QString FileResource::storageTypeString(bool description) const
QUrl FileResource::location() const
{
return mSettings->url();
return mSettings ? mSettings->url() : QUrl();
}
QString FileResource::displayLocation() const
{
return mSettings->displayLocation();
return mSettings ? mSettings->displayLocation() : QString();
}
QString FileResource::displayName() const
{
return mSettings->displayName();
return mSettings ? mSettings->displayName() : QString();
}
QString FileResource::configName() const
{
return mSettings->configName();
return mSettings ? mSettings->configName() : QString();
}
CalEvent::Types FileResource::alarmTypes() const
{
return mSettings->alarmTypes();
return mSettings ? mSettings->alarmTypes() : CalEvent::EMPTY;
}
CalEvent::Types FileResource::enabledTypes() const
{
return mSettings->isValid() ? mSettings->enabledTypes() : CalEvent::EMPTY;
return mSettings && mSettings->isValid() ? mSettings->enabledTypes() : CalEvent::EMPTY;
}
void FileResource::setEnabled(CalEvent::Type type, bool enabled)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
const Changes changes = mSettings->setEnabled(type, enabled);
handleEnabledChange(changes, oldEnabled);
if (mSettings)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
const Changes changes = mSettings->setEnabled(type, enabled);
handleEnabledChange(changes, oldEnabled);
}
}
void FileResource::setEnabled(CalEvent::Types types)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
const Changes changes = mSettings->setEnabled(types);
handleEnabledChange(changes, oldEnabled);
if (mSettings)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
const Changes changes = mSettings->setEnabled(types);
handleEnabledChange(changes, oldEnabled);
}
}
bool FileResource::readOnly() const
{
return mSettings->readOnly();
return mSettings ? mSettings->readOnly() : true;
}
void FileResource::setReadOnly(bool ronly)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setReadOnly(ronly);
if (changes)
if (mSettings)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setReadOnly(ronly);
if (changes)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
}
}
}
int FileResource::writableStatus(CalEvent::Type type) const
{
if (!mSettings->isValid() || mSettings->readOnly())
if (!mSettings || !mSettings->isValid() || mSettings->readOnly())
return -1;
if ((type == CalEvent::EMPTY && !mSettings->enabledTypes())
|| (type != CalEvent::EMPTY && !mSettings->isEnabled(type)))
......@@ -158,65 +169,77 @@ bool FileResource::isWritable(const KAEvent& event) const
bool FileResource::keepFormat() const
{
return mSettings->keepFormat();
return mSettings ? mSettings->keepFormat() : true;
}
void FileResource::setKeepFormat(bool keep)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setKeepFormat(keep);
if (changes)
if (mSettings)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setKeepFormat(keep);
if (changes)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
}
}
}
QColor FileResource::backgroundColour() const
{
return mSettings->backgroundColour();
return mSettings ? mSettings->backgroundColour() : QColor();
}
void FileResource::setBackgroundColour(const QColor& colour)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setBackgroundColour(colour);
if (changes)
if (mSettings)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setBackgroundColour(colour);
if (changes)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
}
}
}
bool FileResource::configIsStandard(CalEvent::Type type) const
{
return mSettings->isStandard(type);
return mSettings ? mSettings->isStandard(type) : false;
}
CalEvent::Types FileResource::configStandardTypes() const
{
return mSettings->standardTypes();
return mSettings ? mSettings->standardTypes() : CalEvent::EMPTY;
}
void FileResource::configSetStandard(CalEvent::Type type, bool standard)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setStandard(type, standard);
if (changes)
if (mSettings)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setStandard(type, standard);
if (changes)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
}
}
}
void FileResource::configSetStandard(CalEvent::Types types)
{
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setStandard(types);
if (changes)
if (mSettings)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
const CalEvent::Types oldEnabled = mSettings->enabledTypes();
Changes changes = mSettings->setStandard(types);
if (changes)
{
handleSettingsChange(changes);
Resources::notifySettingsChanged(this, changes, oldEnabled);
}
}
}
......@@ -258,7 +281,7 @@ void FileResource::editResource(QWidget* dialogParent)
// Note that the location and alarm type cannot be changed.
qCDebug(KALARM_LOG) << "FileResource::editResource: Edited" << dlg->displayName();
setReadOnly(dlg->readOnly());
Changes changes = mSettings->setDisplayName(dlg->displayName());
Changes changes = mSettings ? mSettings->setDisplayName(dlg->displayName()) : NoChange;
if (changes != NoChange)
Resources::notifySettingsChanged(this, changes, enabled);
}
......@@ -294,19 +317,19 @@ bool FileResource::load(bool readThroughCache)
{
qCDebug(KALARM_LOG) << "FileResource::load:" << displayName();
QString errorMessage;
if (!mSettings->isValid())
if (!mSettings || !mSettings->isValid())
{
qCWarning(KALARM_LOG) << "FileResource::load: Resource not configured!" << mSettings->displayName();
qCWarning(KALARM_LOG) << "FileResource::load: Resource not configured!" << displayName();
errorMessage = i18nc("@info", "Resource is not configured.");
}
else if (mStatus == Status::Closed)
qCWarning(KALARM_LOG) << "FileResource::load: Resource closed!" << mSettings->displayName();
qCWarning(KALARM_LOG) << "FileResource::load: Resource closed!" << displayName();
else
{
if (!isEnabled(CalEvent::EMPTY))
{
// Don't load a disabled resource, but mark it as usable (but not loaded).
qCDebug(KALARM_LOG) << "FileResource::load: Resource disabled" << mSettings->displayName();
qCDebug(KALARM_LOG) << "FileResource::load: Resource disabled" << displayName();
mStatus = Status::Ready;
return false;
}
......@@ -335,6 +358,8 @@ bool FileResource::load(bool readThroughCache)
*/
void FileResource::loaded(bool success, QHash<QString, KAEvent>& newEvents, const QString& errorMessage)
{
if (!mSettings)
return;
if (!success)
{
// This is only done when a delayed load fails.
......@@ -420,7 +445,7 @@ bool FileResource::save(QString* errorMessage, bool writeThroughCache, bool forc
bool FileResource::checkSave()
{
QString errorMessage;
if (!mSettings->isValid())
if (!mSettings || !mSettings->isValid())
{
qCWarning(KALARM_LOG) << "FileResource::checkSave: FileResource not configured!" << displayName();
errorMessage = i18nc("@info", "Resource is not configured.");
......@@ -470,7 +495,7 @@ bool FileResource::addEvent(const KAEvent& event)
{
setUpdatedEvents({event}, false);
if (mSettings->isEnabled(CalEvent::ACTIVE))
if (mSettings && mSettings->isEnabled(CalEvent::ACTIVE))
{
// Add this event's command error to the settings.
if (event.category() == CalEvent::ACTIVE
......@@ -517,7 +542,7 @@ bool FileResource::updateEvent(const KAEvent& event, bool saveIfReadOnly)
setUpdatedEvents({event}, false);
// Update command errors held in the settings, if appropriate.
if (mSettings->isEnabled(CalEvent::ACTIVE))
if (mSettings && mSettings->isEnabled(CalEvent::ACTIVE))
handleCommandErrorChange(event);
if (wantSave)
......@@ -546,7 +571,7 @@ bool FileResource::deleteEvent(const KAEvent& event)
{
setDeletedEvents({event});
if (mSettings->isEnabled(CalEvent::ACTIVE))
if (mSettings && mSettings->isEnabled(CalEvent::ACTIVE))
{
QHash<QString, KAEvent::CmdErrType> cmdErrors = mSettings->commandErrors();
if (cmdErrors.remove(event.id()))
......@@ -564,6 +589,8 @@ bool FileResource::deleteEvent(const KAEvent& event)
*/
void FileResource::handleCommandErrorChange(const KAEvent& event)
{
if (!mSettings)
return;
// Update command errors held in the settings, if appropriate.
bool changed = false;
QHash<QString, KAEvent::CmdErrType> cmdErrors = mSettings->commandErrors();
......@@ -612,6 +639,8 @@ bool FileResource::updateStorageFormat(Resource& res)
*/
QString FileResource::identifier() const
{
if (!mSettings)
return {};
return QStringLiteral("FileResource%1").arg(mSettings->id() & ~IdFlag);
}
......@@ -647,7 +676,7 @@ void FileResource::handleSettingsChange(Changes& changes)
if (changes & Enabled)
{
qCDebug(KALARM_LOG) << "FileResource::handleSettingsChange:" << displayId() << "Update enabled status";
if (mSettings->enabledTypes())
if (mSettings && mSettings->enabledTypes())
{
// Alarms are now enabled. Reload the calendar file because,
// although ResourceType retains its record of alarms of disabled
......
......@@ -270,6 +270,9 @@ public:
/** Called to notify the resource that an event's command error has changed. */
void handleCommandErrorChange(const KAEvent&) override;
/** Called when the resource's FileResourceSettings object is about to be destroyed. */
void removeSettings() override { mSettings = nullptr; }
virtual void showProgress(bool) {}
/*-----------------------------------------------------------------------------
......
......@@ -197,6 +197,7 @@ bool FileResourceConfigManager::removeResource(Resource& resource)
const QString configGroup = groupName(groupIndex);
manager->mConfig->deleteGroup(configGroup);
manager->mConfigGroups.remove(groupIndex);
Resources::notifySettingsDestroyed(id); // removing from mResources will destroy settings instance
manager->mResources.remove(id);
return true;
}
......
......@@ -294,6 +294,12 @@ void Resource::handleCommandErrorChange(const KAEvent& event)
mResource->handleCommandErrorChange(event);
}
void Resource::removeSettings()
{
if (!mResource.isNull())
mResource->removeSettings();
}
void Resource::notifyDeletion()
{
if (!mResource.isNull())
......
......@@ -345,6 +345,9 @@ public:
/** Called to notify the resource that an event's command error has changed. */
void handleCommandErrorChange(const KAEvent&);
/** Called when the resource's settings object is about to be destroyed. */
void removeSettings();
/** Must be called to notify the resource that it is being deleted.
* This is to prevent expected errors being displayed to the user.
* @see isBeingDeleted
......
......@@ -570,6 +570,13 @@ void Resources::notifyEventsRemoved(ResourceType* res, const QList<KAEvent>& eve
}
}
void Resources::notifySettingsDestroyed(ResourceId id)
{
Resource r = resource(id);
if (r.isValid())
r.removeSettings();
}
bool Resources::addResource(ResourceType* instance, Resource& resource)
{
if (!instance || instance->id() < 0)
......
......@@ -209,6 +209,9 @@ public:
/** Called by a resource to notify that it has deleted events. */
static void notifyEventsRemoved(ResourceType*, const QList<KAEvent>&);
/** Called by a resource settings instance to notify that it is about to be destructed. */
static void notifySettingsDestroyed(ResourceId);
Q_SIGNALS:
/** Emitted when a resource's settings have changed. */
void settingsChanged(Resource&, ResourceType::Changes);
......
......@@ -387,6 +387,9 @@ public:
/** Called to notify the resource that an event's command error has changed. */
virtual void handleCommandErrorChange(const KAEvent&) = 0;
/** Called when the resource's settings object is about to be destroyed. */
virtual void removeSettings() {}
/** Must be called to notify the resource that it is being deleted.
* This is to prevent expected errors being displayed to the user.
* @see isBeingDeleted
......
......@@ -63,7 +63,7 @@ SingleFileResource::SingleFileResource(FileResourceSettings* settings)
: FileResource(settings)
, mSaveTimer(new QTimer(this))
{
qCDebug(KALARM_LOG) << "SingleFileResource: Starting" << mSettings->displayName();
qCDebug(KALARM_LOG) << "SingleFileResource: Starting" << displayName();
if (!load())
setFailed();
else
......@@ -109,7 +109,7 @@ int SingleFileResource::writableStatus(CalEvent::Type type) const
*/
bool SingleFileResource::updateStorageFmt()
{
if (failed() || readOnly() || enabledTypes() == CalEvent::EMPTY)
if (failed() || readOnly() || enabledTypes() == CalEvent::EMPTY || !mSettings)
return false;
if (!mFileStorage)
{
......@@ -124,7 +124,7 @@ bool SingleFileResource::updateStorageFmt()
return false;
}
qCDebug(KALARM_LOG) << "SingleFileResource::updateStorageFormat: Updating storage for" << mSettings->displayName();
qCDebug(KALARM_LOG) << "SingleFileResource::updateStorageFormat: Updating storage for" << displayName();
mCompatibility = KACalendar::Current;
mVersion = KACalendar::CurrentFormat;
save(nullptr, true, true);
......@@ -167,6 +167,9 @@ bool SingleFileResource::isSaving() const
*/
int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThroughCache, QString& errorMessage)
{
if (!mSettings)
return -1;
newEvents.clear();
if (mDownloadJob)
......@@ -431,13 +434,23 @@ void SingleFileResource::close()
// QEventLoopLocker should ensure that it continues to completion even if
// the destructor for this instance is executed.
if (mSettings->url().isLocalFile())
if (mSettings && mSettings->url().isLocalFile())
KDirWatch::self()->removeFile(mSettings->url().toLocalFile());
mCalendar.clear();
mFileStorage.clear();
mStatus = Status::Closed;
}
/******************************************************************************
* Called when the resource's settings object is about to be destroyed.
*/
void SingleFileResource::removeSettings()
{
if (mSettings && mSettings->url().isLocalFile())
KDirWatch::self()->removeFile(mSettings->url().toLocalFile());
FileResource::removeSettings();
}
/******************************************************************************
* Called from addEvent() to add an event to the resource.
*/
......@@ -464,6 +477,9 @@ bool SingleFileResource::doAddEvent(const KAEvent& event)
*/
bool SingleFileResource::addLoadedEvent(const KCalendarCore::Event::Ptr& kcalEvent)
{
if (!mSettings)
return false;
KAEvent event(kcalEvent);
if (!event.isValid())
{
......@@ -685,8 +701,11 @@ QByteArray SingleFileResource::calculateHash(const QString& fileName) const
*/
void SingleFileResource::saveHash(const QByteArray& hash) const
{
mSettings->setHash(hash.toHex());
mSettings->save();
if (mSettings)
{
mSettings->setHash(hash.toHex());
mSettings->save();
}
}
/******************************************************************************
......@@ -695,6 +714,9 @@ void SingleFileResource::saveHash(const QByteArray& hash) const
*/
void SingleFileResource::localFileChanged(const QString& fileName)
{
if (!mSettings)
return;
if (fileName != mSettings->url().toLocalFile())
return; // not the calendar file for this resource
......@@ -721,7 +743,7 @@ void SingleFileResource::slotDownloadJobResult(KJob* job)
if (mStatus != Status::Closed)
mStatus = Status::Broken;
setLoadFailure(false);
const QString path = mSettings->displayLocation();
const QString path = displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::slotDownloadJobResult:" << displayId() << "Could not load file" << path << job->errorString();
errorMessage = xi18nc("@info", "Could not load file <filename>%1</filename>. (%2)", path, job->errorString());
success = false;
......@@ -761,7 +783,7 @@ void SingleFileResource::slotUploadJobResult(KJob* job)
{
if (mStatus != Status::Closed)
mStatus = Status::Broken;
const QString path = mSettings->displayLocation();
const QString path = displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::slotDownloadJobResult:" << displayId() << "Could not save file" << path << job->errorString();
errorMessage = xi18nc("@info", "Could not save file <filename>%1</filename>. (%2)", path, job->errorString());
success = false;
......@@ -785,7 +807,7 @@ void SingleFileResource::handleSettingsChange(Changes& changes)
qCDebug(KALARM_LOG) << "SingleFileResource::handleSettingsChange:" << displayId();
if (changes & UpdateFormat)
{
if (mSettings->updateFormat())
if (mSettings && mSettings->updateFormat())
{
// This is a request to update the backend calendar storage format
// to the current KAlarm format.
......
......@@ -80,6 +80,9 @@ public:
*/
void close() override;
/** Called when the resource's FileResourceSettings object is about to be destroyed. */
void removeSettings() override;
protected:
/** Update the resource to the current KAlarm storage format. */
bool updateStorageFmt() override;
......
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