Commit f1164fbf authored by Bernd Schmidt's avatar Bernd Schmidt
Browse files

Remove mutexes and other threading leftovers

parent 923f24da
......@@ -126,7 +126,7 @@ Palapeli::Collection::Collection()
//construct puzzle with CollectionStorageComponent
if (!path.isEmpty() && (desktopPath.isEmpty() || QFileInfo(path).lastModified() >= QFileInfo(desktopPath).lastModified()))
{
Palapeli::Puzzle* puzzle = new Palapeli::Puzzle(new Palapeli::CollectionStorageComponent(puzzleGroup, &m_configMutex), path, puzzleId);
Palapeli::Puzzle* puzzle = new Palapeli::Puzzle(new Palapeli::CollectionStorageComponent(puzzleGroup), path, puzzleId);
appendRow(new Item(puzzle));
continue;
}
......@@ -142,9 +142,7 @@ Palapeli::Collection::Collection()
}
/* Moved out of CollectionStorageComponent, where we'd potentially call fdatasync on every
puzzle. */
m_configMutex.lock();
m_config->sync();
m_configMutex.unlock();
}
Palapeli::Collection::~Collection()
......@@ -178,11 +176,9 @@ void Palapeli::Collection::importPuzzle(Palapeli::Puzzle* puzzle)
puzzle->get(Palapeli::PuzzleComponent::ArchiveStorage);
//create the config group for this puzzle (use pseudo-URL to avoid problems
//when the configuration directory is moved)
m_configMutex.lock();
KConfigGroup puzzleGroup(m_group, id);
puzzleGroup.writeEntry("Location", palapeliUrl);
m_config->sync();
m_configMutex.unlock();
//add to the model
appendRow(new Item(puzzle));
}
......@@ -221,10 +217,8 @@ bool Palapeli::Collection::deletePuzzle(const QModelIndex& index)
if (!QFile(puzzle->location()).remove())
return false;
//remove puzzle from config
m_configMutex.lock();
KConfigGroup(m_group, puzzle->identifier()).deleteGroup();
m_config->sync();
m_configMutex.unlock();
//remove puzzle from model and delete it
const int count = rowCount();
for (int row = 0; row < count; ++row)
......
......@@ -58,7 +58,6 @@ namespace Palapeli
KConfig* m_config;
KConfigGroup* m_group;
QMutex m_configMutex;
};
}
......
......@@ -21,10 +21,9 @@
#include <QBuffer>
#include <QFileInfo>
#include <KConfigGroup>
#include <QMutex>
Palapeli::CollectionStorageComponent::CollectionStorageComponent(KConfigGroup* group, QMutex *groupMutex)
: m_group(group), m_groupMutex(groupMutex)
Palapeli::CollectionStorageComponent::CollectionStorageComponent(KConfigGroup* group)
: m_group(group)
{
}
......@@ -46,7 +45,6 @@ Palapeli::PuzzleComponent* Palapeli::CollectionStorageComponent::cast(Type type)
}
//try to serve metadata from cache
const QDateTime mtime = QFileInfo(file).lastModified();
m_groupMutex->lock();
if (m_group->readEntry("ModifyDateTime", QString()) == mtime.toString())
{
//cache is up-to-date
......@@ -58,12 +56,10 @@ Palapeli::PuzzleComponent* Palapeli::CollectionStorageComponent::cast(Type type)
metadata.modifyProtection = m_group->readEntry("ModifyProtection", false);
QByteArray ar = m_group->readEntry("Thumbnail", QByteArray());
metadata.thumbnail.loadFromData(QByteArray::fromBase64 (ar));
m_groupMutex->unlock();
return new Palapeli::MetadataComponent(metadata);
}
else
{
m_groupMutex->unlock();
//read metadata from archive...
const Palapeli::PuzzleComponent* arStorage = puzzle()->get(ArchiveStorage);
if (!arStorage)
......@@ -76,7 +72,6 @@ Palapeli::PuzzleComponent* Palapeli::CollectionStorageComponent::cast(Type type)
const Palapeli::PuzzleMetadata metadata = dynamic_cast<Palapeli::MetadataComponent*>(cMetadata)->metadata;
QBuffer buffer;
metadata.thumbnail.save(&buffer, "PNG");
m_groupMutex->lock();
m_group->writeEntry("Name", metadata.name);
m_group->writeEntry("Comment", metadata.comment);
m_group->writeEntry("Author", metadata.author);
......@@ -84,7 +79,6 @@ Palapeli::PuzzleComponent* Palapeli::CollectionStorageComponent::cast(Type type)
m_group->writeEntry("ModifyProtection", metadata.modifyProtection);
m_group->writeEntry("ModifyDateTime", mtime.toString());
m_group->writeEntry("Thumbnail", buffer.data().toBase64 ());
m_groupMutex->unlock();
return cMetadata;
}
}
......@@ -107,13 +107,12 @@ namespace Palapeli
COMPONENT_SUBCLASS(CollectionStorage)
public:
///Takes ownership of @a group.
CollectionStorageComponent(KConfigGroup* group, QMutex *groupMutex);
CollectionStorageComponent(KConfigGroup* group);
virtual ~CollectionStorageComponent();
Palapeli::PuzzleComponent* cast(Type type) const Q_DECL_OVERRIDE;
private:
KConfigGroup* m_group;
QMutex *m_groupMutex;
};
///This is used by the collection if, instead of an actual puzzle archive,
......
......@@ -18,13 +18,8 @@
#include "puzzle.h"
#include <QAtomicInt>
#include <QAtomicPointer>
#include <QFileInfo>
#include <QHash>
#include <QMutexLocker>
#include <QWaitCondition>
#include <QtConcurrentRun>
//BEGIN Palapeli::PuzzleComponent
......@@ -54,21 +49,17 @@ Palapeli::Puzzle* Palapeli::PuzzleComponent::puzzle() const
//See Private::get() for the whole story.
struct Component
{
QAtomicInt available;
QAtomicPointer<Palapeli::PuzzleComponent> component;
QWaitCondition wait;
Palapeli::PuzzleComponent *component = nullptr;
Component() : available(false), component(0) {}
explicit Component(Palapeli::PuzzleComponent* component) : available(true), component(component) {}
~Component() { wait.wakeAll(); delete component; }
Component() {}
explicit Component(Palapeli::PuzzleComponent* component) : component(component) {}
~Component() { delete component; }
};
struct Palapeli::Puzzle::Private
{
Palapeli::Puzzle* q;
QMutex m_hashMutex; //controls access to m_components
QHash<Palapeli::PuzzleComponent::Type, Component*> m_components;
QAtomicPointer<Palapeli::PuzzleComponent> m_mainComponent;
QMutex m_locationMutex; //controls access to m_location
Palapeli::PuzzleComponent *m_mainComponent;
QString m_location;
QString m_identifier;
......@@ -88,18 +79,15 @@ Palapeli::Puzzle::Private::Private(Palapeli::Puzzle* q, Palapeli::PuzzleComponen
, m_location(location)
, m_identifier(identifier)
{
m_mainComponent.loadAcquire()->m_puzzle = q;
m_mainComponent->m_puzzle = q;
m_components.insert(mainComponent->type(), new Component(mainComponent));
//mutexes not necessary here because concurrent access is impossible in the ctor
}
Palapeli::Puzzle::~Puzzle()
{
d->m_hashMutex.lock();
QHashIterator<Palapeli::PuzzleComponent::Type, Component*> iter(d->m_components);
while (iter.hasNext())
delete iter.next().value();
d->m_hashMutex.unlock();
delete d;
}
......@@ -116,47 +104,16 @@ const Palapeli::PuzzleComponent* Palapeli::Puzzle::get(Palapeli::PuzzleComponent
const Palapeli::PuzzleComponent* Palapeli::Puzzle::Private::get(Palapeli::PuzzleComponent::Type type)
{
bool doRequest = false;
Component* c = 0;
//mutex-secured hash access to check state of component
{
QMutexLocker l(&m_hashMutex);
c = m_components.value(type);
if (!c)
{
//this get() is the first to request this component - create Component
//to mark this component as "work in progress"
m_components.insert(type, c = new Component);
//but release mutex before creating the component
doRequest = true;
}
else if (c->available)
return c->component;
}
//start cast() to create component
if (doRequest)
{
Palapeli::PuzzleComponent* cmp = m_mainComponent.load()->cast(type);
if (cmp)
cmp->m_puzzle = q;
//write access to c need not be mutex-secured because there
//is only one write access ever per component
c->component.fetchAndStoreOrdered(cmp);
c->available.fetchAndStoreOrdered(true);
//notify other waiting threads that the component is available
c->wait.wakeAll();
return cmp;
}
//component has been requested by another worker thread - wait until that
//thread is done (but come back every 1000 ms in case the other thread
//jumped through a concurrent loophole and triggered the waitcondition
//without me listening)
QMutex mutex;
mutex.lock();
while (!c->available)
c->wait.wait(&mutex, 1000);
mutex.unlock();
return c->component;
Component* c = m_components.value(type);
if (c)
return c->component;
m_components.insert(type, c = new Component);
Palapeli::PuzzleComponent* cmp = m_mainComponent->cast(type);
if (cmp)
cmp->m_puzzle = q;
c->component = cmp;
return cmp;
}
QString Palapeli::Puzzle::identifier() const
......@@ -166,13 +123,11 @@ QString Palapeli::Puzzle::identifier() const
QString Palapeli::Puzzle::location() const
{
QMutexLocker l(&d->m_locationMutex);
return d->m_location;
}
void Palapeli::Puzzle::setLocation(const QString& location)
{
QMutexLocker l(&d->m_locationMutex);
d->m_location = location;
}
......@@ -181,17 +136,15 @@ void Palapeli::Puzzle::setMainComponent(Palapeli::PuzzleComponent* component)
if (!component)
return;
//add component
QMutexLocker locker(&d->m_hashMutex);
Component*& c = d->m_components[component->type()];
delete c;
c = new Component(component);
d->m_mainComponent.fetchAndStoreOrdered(component);
d->m_mainComponent = component;
}
void Palapeli::Puzzle::dropComponent(Palapeli::PuzzleComponent::Type type)
{
//DO NEVER EVER USE THIS FUNCTION! THIS FUNCTION IS PURELY DANGEROUS. STUFF WILL BREAK.
QMutexLocker locker(&d->m_hashMutex);
Component*& c = d->m_components[type];
delete c;
c = 0;
......
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