Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/Platform.cpp

Issue 29706560: Issue 5179 - Implement asynchronous executor with a controllable lifetime (Closed) Base URL: https://github.com/adblockplus/libadblockplus.git
Patch Set: Created Feb. 23, 2018, 10:29 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: src/Platform.cpp
diff --git a/src/Platform.cpp b/src/Platform.cpp
index 17cc7ab37930df839c1de2500b4c07df5781597a..e53ae974a75efeec09e8ac42adc629adda5a47b6 100644
--- a/src/Platform.cpp
+++ b/src/Platform.cpp
@@ -18,6 +18,7 @@
#include <AdblockPlus/JsEngine.h>
#include <AdblockPlus/FilterEngine.h>
#include <AdblockPlus/DefaultLogSystem.h>
+#include <AdblockPlus/AsyncExecutor.h>
#include "DefaultTimer.h"
#include "DefaultWebRequest.h"
#include "DefaultFileSystem.h"
@@ -27,11 +28,6 @@ using namespace AdblockPlus;
namespace
{
- void DummyScheduler(const AdblockPlus::SchedulerTask& task)
- {
- std::thread(task).detach();
- }
-
template<typename T>
void ValidatePlatformCreationParameter(const std::unique_ptr<T>& param, const char* paramName)
{
@@ -121,11 +117,35 @@ void Platform::WithLogSystem(const WithLogSystemCallback& callback)
namespace
{
+ class SharedAsyncExecutor {
sergei 2018/02/23 11:16:42 It's rather a compatibility thing to simplify the
+ public:
+ SharedAsyncExecutor()
+ : executor(new AsyncExecutor())
+ {
+ }
+ void Dispatch(const std::function<void()>& task) {
+ std::lock_guard<std::mutex> lock(asyncExecutorMutex);
+ if (!executor)
+ return;
+ executor->Dispatch(task);
+ }
+ void Invalidate() {
+ std::unique_ptr<AsyncExecutor> tmp;
sergei 2018/02/23 11:16:42 JIC, it's needed to prevent dead locks. If `execut
+ {
+ std::lock_guard<std::mutex> lock(asyncExecutorMutex);
+ tmp = move(executor);
+ }
+ }
+ private:
+ std::mutex asyncExecutorMutex;
+ std::unique_ptr<AsyncExecutor> executor;
+ };
+
class DefaultPlatform : public Platform
{
public:
- typedef std::shared_ptr<Scheduler> AsyncExecutorPtr;
- explicit DefaultPlatform(const AsyncExecutorPtr& asyncExecutor, CreationParameters&& creationParams)
+ typedef std::shared_ptr<SharedAsyncExecutor> AsyncExecutorPtr;
+ explicit DefaultPlatform(const std::shared_ptr<SharedAsyncExecutor>& asyncExecutor, CreationParameters&& creationParams)
hub 2018/02/23 22:59:35 Since there is `typedef std::shared_ptr<SharedAsyn
sergei 2018/03/01 11:18:37 Fixed, I somehow forgot about it ))
: Platform(std::move(creationParams)), asyncExecutor(asyncExecutor)
{
}
@@ -137,13 +157,13 @@ namespace
void WithLogSystem(const WithLogSystemCallback&) override;
private:
- AsyncExecutorPtr asyncExecutor;
+ std::shared_ptr<SharedAsyncExecutor> asyncExecutor;
std::recursive_mutex interfacesMutex;
};
DefaultPlatform::~DefaultPlatform()
{
- asyncExecutor.reset();
+ asyncExecutor->Invalidate();
LogSystemPtr tmpLogSystem;
TimerPtr tmpTimer;
FileSystemPtr tmpFileSystem;
@@ -184,18 +204,6 @@ namespace
Scheduler DefaultPlatformBuilder::GetDefaultAsyncExecutor()
{
- if (!defaultScheduler)
- {
- asyncExecutor = std::make_shared<Scheduler>(::DummyScheduler);
- std::weak_ptr<Scheduler> weakExecutor = asyncExecutor;
- defaultScheduler = [weakExecutor](const SchedulerTask& task)
- {
- if (auto executor = weakExecutor.lock())
- {
- (*executor)(task);
- }
- };
- }
return defaultScheduler;
}
@@ -223,6 +231,11 @@ void DefaultPlatformBuilder::CreateDefaultLogSystem()
std::unique_ptr<Platform> DefaultPlatformBuilder::CreatePlatform()
{
+ auto sharedAsyncExecutor = std::make_shared<SharedAsyncExecutor>();
+ defaultScheduler = [sharedAsyncExecutor](const SchedulerTask& task)
+ {
+ sharedAsyncExecutor->Dispatch(task);
+ };
if (!logSystem)
CreateDefaultLogSystem();
if (!timer)
@@ -232,7 +245,6 @@ std::unique_ptr<Platform> DefaultPlatformBuilder::CreatePlatform()
if (!webRequest)
CreateDefaultWebRequest();
- std::unique_ptr<Platform> platform(new DefaultPlatform(asyncExecutor, std::move(*this)));
- asyncExecutor.reset();
+ std::unique_ptr<Platform> platform(new DefaultPlatform(sharedAsyncExecutor, std::move(*this)));
return platform;
}

Powered by Google App Engine
This is Rietveld