Accepting request 478097 from KDE:Applications

KDE Applications 16.12.3

OBS-URL: https://build.opensuse.org/request/show/478097
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/akonadi-server?expand=0&rev=26
This commit is contained in:
Dominique Leuenberger 2017-03-14 23:50:22 +00:00 committed by Git OBS Bridge
commit 9318943020
5 changed files with 33 additions and 304 deletions

View File

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:bf40309f36bea813341187f72d55546eeabe0538657ea9a665a3156393514ab3
size 1036664

3
akonadi-16.12.3.tar.xz Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:fc48a200c27331d5a9ac6bfb8d7d61f3c4240f133bb23c996c78493f24644b03
size 1037568

View File

@ -1,3 +1,31 @@
-------------------------------------------------------------------
Thu Mar 9 18:07:28 UTC 2017 - alarrosa@suse.com
- Drop fix-itemretriever-in-case-of-concurrent-requests.patch, it's
already merged upstream
-------------------------------------------------------------------
Thu Mar 9 12:33:17 CET 2017 - lbeltrame@kde.org
- Update to 16.12.3
* New bugfix release
* For more details please see:
* https://www.kde.org/announcements/announce-applications-16.12.3.php
- Changes since 16.12.2:
* Fix ServerManager going into Broken state when called before going to the event loop.
* Fix crash when Connection is closed while ItemRetriever is running (kde#374734)
* DataStream: throw exception if device is null (kde#376385)
* Don't export internal classes when not building autotests.
* Remove unused included cpp file from autotest.
* Fix ItemRetriever in case of concurrent requests for the same item(s)
* no need to generate the header twice, we can use the one from src/server here
* Revert unwanted changes in previous commit
* Implement "TODO: Qt 5: Use QDir::removeRecursively"
* Fix runtime warning at the end of itemretrievertest.
* Show collection id if collection is valid
* Add more debug
-------------------------------------------------------------------
Fri Feb 24 13:14:48 UTC 2017 - lbeltrame@kde.org

View File

@ -18,10 +18,10 @@
%define rname akonadi
%define kf5_version 5.26.0
# Latest stable Applications (e.g. 16.08 in KA, but 16.12.2 in KUA)
# Latest stable Applications (e.g. 16.08 in KA, but 16.12.3 in KUA)
%{!?_kapp_version: %global _kapp_version %(echo %{version}| awk -F. '{print $1"."$2}')}
Name: akonadi-server
Version: 16.12.2
Version: 16.12.3
Release: 0
Summary: PIM Storage Service
License: LGPL-2.1+
@ -31,8 +31,6 @@ Source: %{rname}-%{version}.tar.xz
Source99: akonadi-server-rpmlintrc
# PATCH-FIX-UPSTREAM handle-mysql-process-crashes-gracefully.patch
Patch0: handle-mysql-process-crashes-gracefully.patch
# PATCH-FIX-UPSTREAM fix-itemretriever-in-case-of-concurrent-requests.patch - Fixes hangs when accessing folders
Patch1: fix-itemretriever-in-case-of-concurrent-requests.patch
%if 0%{?suse_version} > 1325
BuildRequires: libboost_headers-devel
%else
@ -172,7 +170,6 @@ service.
%prep
%setup -q -n %{rname}-%{version}
%patch0 -p1
%patch1 -p1
%build
%cmake_kf5 -d build -- -DINSTALL_QSQLITE_IN_QT_PREFIX=TRUE -DQT_PLUGINS_DIR=%{_kf5_plugindir}

View File

@ -1,296 +0,0 @@
From d42f94701f38c26cf3439727b50ee73cf0d09bee Mon Sep 17 00:00:00 2001
From: David Faure <faure@kde.org>
Date: Wed, 15 Feb 2017 08:43:09 +0100
Subject: Fix ItemRetriever in case of concurrent requests for the same item(s)
Summary:
- ItemRetrievalManager must emit requestFinished() for those other requests,
otherwise the list of pending requests in ItemRetriever is never emptied
- ItemRetriever must not assume that a signal being emitted by
ItemRetrievalManager is necessarily about the request it's waiting for, it
could be for another one. So it must first check in its list of pending
requests to determine whether it should react or not.
With multithreaded unittest, checked for races with clang+tsan.
(There is one race, the connect to ItemRetrievalRequest vs the emit
in other threads, we should lock mLock before connect...)
Test Plan: new unittest
Reviewers: dvratil
Reviewed By: dvratil
Subscribers: #kde_pim
Tags: #kde_pim
Differential Revision: https://phabricator.kde.org/D4618
---
autotests/server/itemretrievertest.cpp | 174 ++++++++++++++++++++--------
src/server/storage/itemretrievalmanager.cpp | 1 +
src/server/storage/itemretriever.cpp | 21 ++--
3 files changed, 135 insertions(+), 61 deletions(-)
diff --git a/autotests/server/itemretrievertest.cpp b/autotests/server/itemretrievertest.cpp
index fb4468d..1206b35 100644
--- a/autotests/server/itemretrievertest.cpp
+++ b/autotests/server/itemretrievertest.cpp
@@ -20,6 +20,7 @@
#include <QObject>
#include <QTest>
#include <QTimer>
+#include <QMutex>
#include "storage/itemretriever.h"
#include "storage/itemretrievaljob.h"
@@ -144,6 +145,52 @@ private:
QMultiHash<qint64, JobResult> mJobResults;
};
+using RequestedParts = QVector<QByteArray /* FQ name */>;
+
+class ClientThread : public QThread
+{
+public:
+ ClientThread(Entity::Id itemId, const RequestedParts &requestedParts)
+ : m_itemId(itemId), m_requestedParts(requestedParts)
+ {}
+
+ void run() Q_DECL_OVERRIDE
+ {
+ // ItemRetriever should...
+ ItemRetriever retriever;
+ retriever.setItem(m_itemId);
+ retriever.setRetrieveParts(m_requestedParts);
+ QSignalSpy spy(&retriever, &ItemRetriever::itemsRetrieved);
+
+ const bool success = retriever.exec();
+
+ QMutexLocker lock(&m_mutex);
+ m_results.success = success;
+ m_results.signalsCount = spy.count();
+ if (m_results.signalsCount > 0) {
+ m_results.emittedItems = spy.at(0).at(0).value<QList<qint64>>();
+ }
+ }
+
+ struct Results
+ {
+ bool success;
+ int signalsCount;
+ QList<qint64> emittedItems;
+ };
+ Results results() const {
+ QMutexLocker lock(&m_mutex);
+ return m_results;
+ }
+
+private:
+ const Entity::Id m_itemId;
+ const RequestedParts m_requestedParts;
+
+ mutable QMutex m_mutex; // protects results below
+ Results m_results;
+};
+
class ItemRetrieverTest : public QObject
{
Q_OBJECT
@@ -151,7 +198,6 @@ class ItemRetrieverTest : public QObject
using ExistingParts = QVector<QPair<QByteArray /* name */, QByteArray /* data */>>;
using AvailableParts = QVector<QPair<QByteArray /* name */, QByteArray /* data */>>;
- using RequestedParts = QVector<QByteArray /* FQ name */>;
public:
ItemRetrieverTest()
@@ -252,63 +298,89 @@ private Q_SLOTS:
// Setup
- DbInitializer dbInitializer;
- FakeItemRetrievalJobFactory factory(dbInitializer);
- ItemRetrievalManager mgr(&factory);
- QTest::qWait(100);
-
- // Given a PimItem with existing parts
- Resource res = dbInitializer.createResource("testresource");
- Collection col = dbInitializer.createCollection("col1");
- PimItem item = dbInitializer.createItem("1", col);
- Q_FOREACH (const auto &existingPart, existingParts) {
- dbInitializer.createPart(item.id(), existingPart.first, existingPart.second);
- }
+ for (int step = 0; step < 2; ++step) {
+ DbInitializer dbInitializer;
+ FakeItemRetrievalJobFactory factory(dbInitializer);
+ ItemRetrievalManager mgr(&factory);
+ QTest::qWait(100);
+
+ // Given a PimItem with existing parts
+ Resource res = dbInitializer.createResource("testresource");
+ Collection col = dbInitializer.createCollection("col1");
+
+ // step 0: do it in the main thread, for easier debugging
+ PimItem item = dbInitializer.createItem("1", col);
+ Q_FOREACH (const auto &existingPart, existingParts) {
+ dbInitializer.createPart(item.id(), existingPart.first, existingPart.second);
+ }
- Q_FOREACH (const auto &availablePart, availableParts) {
- factory.addJobResult(item.id(), availablePart.first, availablePart.second);
- }
+ Q_FOREACH (const auto &availablePart, availableParts) {
+ factory.addJobResult(item.id(), availablePart.first, availablePart.second);
+ }
- // ItemRetriever should...
- ItemRetriever retriever;
- retriever.setItem(item.id());
- retriever.setRetrieveParts(requestedParts);
- QSignalSpy spy(&retriever, &ItemRetriever::itemsRetrieved);
+ if (step == 0) {
+ ClientThread thread(item.id(), requestedParts);
+ thread.run();
+
+ const ClientThread::Results results = thread.results();
+ // ItemRetriever should ... succeed
+ QVERIFY(results.success);
+ // Emit exactly one signal ...
+ QCOMPARE(results.signalsCount, expectedSignals);
+ // ... with that one item
+ if (expectedSignals > 0) {
+ QCOMPARE(results.emittedItems, QList<qint64>{ item.id() });
+ }
- // Succeed
- QVERIFY(retriever.exec());
- // Run exactly one retrieval job
- QCOMPARE(factory.jobsCount(), expectedRetrievalJobs);
- // Emit exactly one signal ...
- QCOMPARE(spy.count(), expectedSignals);
- // ... with that one item
- if (expectedSignals > 0) {
- QCOMPARE(spy.at(0).at(0).value<QList<qint64>>(), QList<qint64>{ item.id() });
- }
+ // Check that the factory had exactly one retrieval job
+ QCOMPARE(factory.jobsCount(), expectedRetrievalJobs);
- // and the part exists in the DB
- const auto parts = item.parts();
- QCOMPARE(parts.count(), expectedParts);
- Q_FOREACH (const Part &dbPart, item.parts()) {
- const QString fqname = dbPart.partType().ns() + QLatin1Char(':') + dbPart.partType().name();
- if (!requestedParts.contains(fqname.toLatin1())) {
- continue;
+ } else {
+ QVector<ClientThread *> threads;
+ for (int i = 0; i < 20; ++i) {
+ threads.append(new ClientThread(item.id(), requestedParts));
+ }
+ for (int i = 0; i < threads.size(); ++i) {
+ threads.at(i)->start();
+ }
+ for (int i = 0; i < threads.size(); ++i) {
+ threads.at(i)->wait();
+ }
+ for (int i = 0; i < threads.size(); ++i) {
+ const ClientThread::Results results = threads.at(i)->results();
+ QVERIFY(results.success);
+ QCOMPARE(results.signalsCount, expectedSignals);
+ if (expectedSignals > 0) {
+ QCOMPARE(results.emittedItems, QList<qint64>{ item.id() });
+ }
+ }
+ qDeleteAll(threads);
}
- auto it = std::find_if(availableParts.constBegin(), availableParts.constEnd(),
- [dbPart](const QPair<QByteArray, QByteArray> &p) {
- return dbPart.partType().name().toLatin1() == p.first;
- });
- if (it == availableParts.constEnd()) {
- it = std::find_if(existingParts.constBegin(), existingParts.constEnd(),
- [fqname](const QPair<QByteArray, QByteArray> &p) {
- return fqname.toLatin1() == p.first;
- });
- QVERIFY(it != existingParts.constEnd());
- }
+ // Check that the parts now exist in the DB
+ const auto parts = item.parts();
+ QCOMPARE(parts.count(), expectedParts);
+ Q_FOREACH (const Part &dbPart, item.parts()) {
+ const QString fqname = dbPart.partType().ns() + QLatin1Char(':') + dbPart.partType().name();
+ if (!requestedParts.contains(fqname.toLatin1())) {
+ continue;
+ }
+
+ auto it = std::find_if(availableParts.constBegin(), availableParts.constEnd(),
+ [dbPart](const QPair<QByteArray, QByteArray> &p) {
+ return dbPart.partType().name().toLatin1() == p.first;
+ });
+ if (it == availableParts.constEnd()) {
+ it = std::find_if(existingParts.constBegin(), existingParts.constEnd(),
+ [fqname](const QPair<QByteArray, QByteArray> &p) {
+ return fqname.toLatin1() == p.first;
+ });
+ QVERIFY(it != existingParts.constEnd());
+ }
- QCOMPARE(dbPart.data(), it->second);
- QCOMPARE(dbPart.datasize(), it->second.size());
+ QCOMPARE(dbPart.data(), it->second);
+ QCOMPARE(dbPart.datasize(), it->second.size());
+ }
}
}
};
diff --git a/src/server/storage/itemretrievalmanager.cpp b/src/server/storage/itemretrievalmanager.cpp
index 65f5e78..382bb9e 100644
--- a/src/server/storage/itemretrievalmanager.cpp
+++ b/src/server/storage/itemretrievalmanager.cpp
@@ -232,6 +232,7 @@ void ItemRetrievalManager::retrievalJobFinished(ItemRetrievalRequest *request, c
qCDebug(AKONADISERVER_LOG) << "someone else requested item" << request->ids << "as well, marking as processed";
(*it)->errorMsg = errorMsg;
(*it)->processed = true;
+ Q_EMIT requestFinished(*it);
it = mPendingRequests[request->resourceId].erase(it);
} else {
++it;
diff --git a/src/server/storage/itemretriever.cpp b/src/server/storage/itemretriever.cpp
index 0ae3e91..c57c860 100644
--- a/src/server/storage/itemretriever.cpp
+++ b/src/server/storage/itemretriever.cpp
@@ -317,17 +317,18 @@ bool ItemRetriever::exec()
QEventLoop eventLoop;
connect(ItemRetrievalManager::instance(), &ItemRetrievalManager::requestFinished,
this, [&](ItemRetrievalRequest *finishedRequest) {
- if (!finishedRequest->errorMsg.isEmpty()) {
- mLastError = finishedRequest->errorMsg.toUtf8();
- eventLoop.exit(1);
- } else {
- requests.removeOne(finishedRequest);
- Q_EMIT itemsRetrieved(finishedRequest->ids);
- if (requests.isEmpty()) {
- eventLoop.quit();
- }
+ if (requests.removeOne(finishedRequest)) {
+ if (!finishedRequest->errorMsg.isEmpty()) {
+ mLastError = finishedRequest->errorMsg.toUtf8();
+ eventLoop.exit(1);
+ } else {
+ Q_EMIT itemsRetrieved(finishedRequest->ids);
+ if (requests.isEmpty()) {
+ eventLoop.quit();
}
- }, Qt::UniqueConnection);
+ }
+ }
+ }, Qt::UniqueConnection);
auto it = requests.begin();
while (it != requests.end()) {
--
cgit v0.11.2