summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian Mollekopf <chrigi_1@fastmail.fm>2018-07-25 17:21:22 +0200
committerChristian Mollekopf <chrigi_1@fastmail.fm>2018-07-25 17:21:22 +0200
commit898f35f2982e86f95c7fe061aa5e697c771a0d47 (patch)
tree95425299b4c4668258707de9cc149ba6f43add10
parenta9cd61f1baafe09fd20ffb6ba1c6728a8792b344 (diff)
downloadsink-898f35f2982e86f95c7fe061aa5e697c771a0d47.tar.gz
sink-898f35f2982e86f95c7fe061aa5e697c771a0d47.zip
Avoid the socket probing and move the shutdown logic into
resourceaccess. The problem was (as excercised by the last test in resourcecontroltest), that in this scenario we would: * trigger a synchronization that starts the resource, and then goes into a loop trying to connecting (KAsync::wait -> singleshot timer) * trigger a shutdown that would probe for the socket, not find it, and thus do nothing. * exit the testfunction, which somehow stops qtimer processing, meaning we are stuck in KAsync::wait. For now this is fixed by simply not probing for the socket.
-rw-r--r--common/resourceaccess.cpp5
-rw-r--r--common/resourceaccess.h6
-rw-r--r--common/resourcecontrol.cpp49
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/resourcecontroltest.cpp83
5 files changed, 114 insertions, 30 deletions
diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp
index 2fa753d..033acc9 100644
--- a/common/resourceaccess.cpp
+++ b/common/resourceaccess.cpp
@@ -424,6 +424,11 @@ KAsync::Job<void> ResourceAccess::sendSecret(const QString &secret)
424 return sendCommand(Sink::Commands::SecretCommand, fbb); 424 return sendCommand(Sink::Commands::SecretCommand, fbb);
425} 425}
426 426
427KAsync::Job<void> ResourceAccess::shutdown()
428{
429 return sendCommand(Sink::Commands::ShutdownCommand);
430}
431
427void ResourceAccess::open() 432void ResourceAccess::open()
428{ 433{
429 if (d->socket && d->socket->isValid()) { 434 if (d->socket && d->socket->isValid()) {
diff --git a/common/resourceaccess.h b/common/resourceaccess.h
index ea3329d..e791236 100644
--- a/common/resourceaccess.h
+++ b/common/resourceaccess.h
@@ -86,6 +86,11 @@ public:
86 return KAsync::null<void>(); 86 return KAsync::null<void>();
87 } 87 }
88 88
89 virtual KAsync::Job<void> shutdown()
90 {
91 return KAsync::null<void>();
92 }
93
89 int getResourceStatus() const 94 int getResourceStatus() const
90 { 95 {
91 return mResourceStatus; 96 return mResourceStatus;
@@ -128,6 +133,7 @@ public:
128 sendInspectionCommand(int inspectionType,const QByteArray &inspectionId, const QByteArray &domainType, const QByteArray &entityId, const QByteArray &property, const QVariant &expecedValue) Q_DECL_OVERRIDE; 133 sendInspectionCommand(int inspectionType,const QByteArray &inspectionId, const QByteArray &domainType, const QByteArray &entityId, const QByteArray &property, const QVariant &expecedValue) Q_DECL_OVERRIDE;
129 KAsync::Job<void> sendFlushCommand(int flushType, const QByteArray &flushId) Q_DECL_OVERRIDE; 134 KAsync::Job<void> sendFlushCommand(int flushType, const QByteArray &flushId) Q_DECL_OVERRIDE;
130 KAsync::Job<void> sendSecret(const QString &secret) Q_DECL_OVERRIDE; 135 KAsync::Job<void> sendSecret(const QString &secret) Q_DECL_OVERRIDE;
136 KAsync::Job<void> shutdown() Q_DECL_OVERRIDE;
131 /** 137 /**
132 * Tries to connect to server, and returns a connected socket on success. 138 * Tries to connect to server, and returns a connected socket on success.
133 */ 139 */
diff --git a/common/resourcecontrol.cpp b/common/resourcecontrol.cpp
index 5e8e09a..ea660c5 100644
--- a/common/resourcecontrol.cpp
+++ b/common/resourcecontrol.cpp
@@ -37,38 +37,27 @@ KAsync::Job<void> ResourceControl::shutdown(const QByteArray &identifier)
37 SinkTrace() << "shutdown " << identifier; 37 SinkTrace() << "shutdown " << identifier;
38 auto time = QSharedPointer<QTime>::create(); 38 auto time = QSharedPointer<QTime>::create();
39 time->start(); 39 time->start();
40 return ResourceAccess::connectToServer(identifier) 40
41 .then<void, QSharedPointer<QLocalSocket>>( 41 auto resourceAccess = ResourceAccessFactory::instance().getAccess(identifier, ResourceConfig::getResourceType(identifier));
42 [identifier, time](const KAsync::Error &error, QSharedPointer<QLocalSocket> socket) { 42 return resourceAccess->shutdown()
43 if (error) { 43 .addToContext(resourceAccess)
44 SinkTrace() << "Resource is already closed."; 44 .then<void>([resourceAccess, time](KAsync::Future<void> &future) {
45 // Resource isn't started, nothing to shutdown 45 SinkTrace() << "Shutdown command complete, waiting for shutdown." << Log::TraceTime(time->elapsed());
46 return KAsync::null(); 46 if (!resourceAccess->isReady()) {
47 future.setFinished();
48 return;
49 }
50 auto guard = new QObject;
51 QObject::connect(resourceAccess.data(), &ResourceAccess::ready, guard, [&future, guard](bool ready) {
52 if (!ready) {
53 //Protect against callback getting called twice.
54 delete guard;
55 future.setFinished();
47 } 56 }
48 // We can't currently reuse the socket
49 socket->close();
50 auto resourceAccess = ResourceAccessFactory::instance().getAccess(identifier, ResourceConfig::getResourceType(identifier));
51 resourceAccess->open();
52 return resourceAccess->sendCommand(Sink::Commands::ShutdownCommand)
53 .addToContext(resourceAccess)
54 .then<void>([resourceAccess, time](KAsync::Future<void> &future) {
55 SinkTrace() << "Shutdown command complete, waiting for shutdown." << Log::TraceTime(time->elapsed());
56 if (!resourceAccess->isReady()) {
57 future.setFinished();
58 return;
59 }
60 auto guard = new QObject;
61 QObject::connect(resourceAccess.data(), &ResourceAccess::ready, guard, [&future, guard](bool ready) {
62 if (!ready) {
63 //Protect against callback getting called twice.
64 delete guard;
65 future.setFinished();
66 }
67 });
68 }).then([time] {
69 SinkTrace() << "Shutdown complete." << Log::TraceTime(time->elapsed());
70 });
71 }); 57 });
58 }).then([time] {
59 SinkTrace() << "Shutdown complete." << Log::TraceTime(time->elapsed());
60 });
72} 61}
73 62
74KAsync::Job<void> ResourceControl::start(const QByteArray &identifier) 63KAsync::Job<void> ResourceControl::start(const QByteArray &identifier)
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 2706d5b..3bbbe37 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -55,6 +55,7 @@ auto_tests (
55 notificationtest 55 notificationtest
56 entitystoretest 56 entitystoretest
57 upgradetest 57 upgradetest
58 resourcecontroltest
58) 59)
59 60
60if (WIN32) 61if (WIN32)
diff --git a/tests/resourcecontroltest.cpp b/tests/resourcecontroltest.cpp
new file mode 100644
index 0000000..e66f444
--- /dev/null
+++ b/tests/resourcecontroltest.cpp
@@ -0,0 +1,83 @@
1#include <QtTest>
2
3#include "dummyresource/resourcefactory.h"
4#include "resourcecontrol.h"
5#include "store.h"
6#include "testutils.h"
7#include "test.h"
8#include "resourceconfig.h"
9
10/**
11 * Test starting and stopping of resources.
12 */
13class ResourceControlTest : public QObject
14{
15 Q_OBJECT
16
17 KAsync::Job<bool> socketIsAvailable(const QByteArray &identifier)
18 {
19 return Sink::ResourceAccess::connectToServer(identifier)
20 .then<void, QSharedPointer<QLocalSocket>>(
21 [&](const KAsync::Error &error, QSharedPointer<QLocalSocket> socket) {
22 if (error) {
23 return KAsync::value(false);
24 }
25 socket->close();
26 return KAsync::value(true);
27 });
28
29 }
30
31 bool blockingSocketIsAvailable(const QByteArray &identifier)
32 {
33 auto job = socketIsAvailable(identifier);
34 auto future = job.exec();
35 future.waitForFinished();
36 return future.value();
37 }
38
39private slots:
40
41 void initTestCase()
42 {
43 Sink::Test::initTest();
44 auto factory = Sink::ResourceFactory::load("sink.dummy");
45 QVERIFY(factory);
46 ::DummyResource::removeFromDisk("sink.dummy.instance1");
47 ResourceConfig::addResource("sink.dummy.instance1", "sink.dummy");
48 ::DummyResource::removeFromDisk("sink.dummy.instance2");
49 ResourceConfig::addResource("sink.dummy.instance2", "sink.dummy");
50 }
51
52 void testResourceStart()
53 {
54 VERIFYEXEC(Sink::ResourceControl::start("sink.dummy.instance1"));
55 QVERIFY(blockingSocketIsAvailable("sink.dummy.instance1"));
56 }
57
58 void testResourceShutdown()
59 {
60 QVERIFY(!blockingSocketIsAvailable("sink.dummy.instance2"));
61 VERIFYEXEC(Sink::ResourceControl::start("sink.dummy.instance2"));
62 QVERIFY(blockingSocketIsAvailable("sink.dummy.instance2"));
63 VERIFYEXEC(Sink::ResourceControl::shutdown("sink.dummy.instance2"));
64 QVERIFY(!blockingSocketIsAvailable("sink.dummy.instance2"));
65 }
66
67 //This will produce a race where the synchronize command starts the resource,
68 //the shutdown command doesn't shutdown because it doesn't realize that the resource is up,
69 //and the resource ends up getting started, but doing nothing.
70 void testResourceShutdownAfterStartByCommand()
71 {
72 QVERIFY(!blockingSocketIsAvailable("sink.dummy.instance2"));
73 auto future = Sink::Store::synchronize(Sink::SyncScope{}.resourceFilter("sink.dummy.instance2")).exec();
74
75 VERIFYEXEC(Sink::ResourceControl::shutdown("sink.dummy.instance2"));
76
77 QVERIFY(!blockingSocketIsAvailable("sink.dummy.instance2"));
78 }
79
80};
81
82QTEST_MAIN(ResourceControlTest)
83#include "resourcecontroltest.moc"