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

Side by Side Diff: src/Platform.cpp

Issue 29543810: Issue 5118 - Lock the platform interfaces before use (Closed) Base URL: https://hg.adblockplus.org/libadblockplus/
Patch Set: Review feedback Created Sept. 13, 2017, 7:49 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « src/JsEngine.cpp ('k') | src/WebRequestJsObject.cpp » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 77 matching lines...) Expand 10 before | Expand all | Expand 10 after
88 onCreated(*filterEngine); 88 onCreated(*filterEngine);
89 }, parameters); 89 }, parameters);
90 } 90 }
91 91
92 FilterEngine& Platform::GetFilterEngine() 92 FilterEngine& Platform::GetFilterEngine()
93 { 93 {
94 CreateFilterEngineAsync(); 94 CreateFilterEngineAsync();
95 return *std::shared_future<FilterEnginePtr>(filterEngine).get(); 95 return *std::shared_future<FilterEnginePtr>(filterEngine).get();
96 } 96 }
97 97
98 ITimer& Platform::GetTimer() 98 void Platform::WithTimer(const WithTimerCallback& callback)
99 { 99 {
100 return *timer; 100 if (timer && callback)
101 callback(*timer);
101 } 102 }
102 103
103 IFileSystem& Platform::GetFileSystem() 104 void Platform::WithFileSystem(const WithFileSystemCallback& callback)
104 { 105 {
105 return *fileSystem; 106 if (fileSystem && callback)
107 callback(*fileSystem);
106 } 108 }
107 109
108 IWebRequest& Platform::GetWebRequest() 110 void Platform::WithWebRequest(const WithWebRequestCallback& callback)
109 { 111 {
110 return *webRequest; 112 if (webRequest && callback)
113 callback(*webRequest);
111 } 114 }
112 115
113 LogSystem& Platform::GetLogSystem() 116 void Platform::WithLogSystem(const WithLogSystemCallback& callback)
114 { 117 {
115 return *logSystem; 118 if (logSystem && callback)
119 callback(*logSystem);
116 } 120 }
117 121
118 namespace 122 namespace
119 { 123 {
120 class DefaultPlatform : public Platform 124 class DefaultPlatform : public Platform
121 { 125 {
122 public: 126 public:
123 typedef std::shared_ptr<Scheduler> AsyncExecutorPtr; 127 typedef std::shared_ptr<Scheduler> AsyncExecutorPtr;
124 explicit DefaultPlatform(const AsyncExecutorPtr& asyncExecutor, CreationPara meters&& creationParams) 128 explicit DefaultPlatform(const AsyncExecutorPtr& asyncExecutor, CreationPara meters&& creationParams)
125 : Platform(std::move(creationParams)), asyncExecutor(asyncExecutor) 129 : Platform(std::move(creationParams)), asyncExecutor(asyncExecutor)
126 { 130 {
127 } 131 }
128 ~DefaultPlatform() 132 ~DefaultPlatform();
129 { 133
130 asyncExecutor.reset(); 134 virtual void WithTimer(const WithTimerCallback&) override;
sergei 2017/09/13 20:06:37 Could you please remove the `virtual` keyword here
hub 2017/09/13 20:17:51 Done.
131 } 135 virtual void WithFileSystem(const WithFileSystemCallback&) override;
136 virtual void WithWebRequest(const WithWebRequestCallback&) override;
137 virtual void WithLogSystem(const WithLogSystemCallback&) override;
138
132 private: 139 private:
133 AsyncExecutorPtr asyncExecutor; 140 AsyncExecutorPtr asyncExecutor;
141 std::recursive_mutex interfacesMutex;
134 }; 142 };
143
144 DefaultPlatform::~DefaultPlatform()
145 {
146 asyncExecutor.reset();
147 LogSystemPtr tmpLogSystem;
148 TimerPtr tmpTimer;
149 FileSystemPtr tmpFileSystem;
150 WebRequestPtr tmpWebRequest;
151 {
152 std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
153 tmpLogSystem = std::move(logSystem);
154 tmpTimer = std::move(timer);
155 tmpFileSystem = std::move(fileSystem);
156 tmpWebRequest = std::move(webRequest);
157 }
158 tmpLogSystem.reset();
159 tmpTimer.reset();
160 tmpFileSystem.reset();
161 tmpWebRequest.reset();
sergei 2017/09/13 20:06:37 I think this calls of reset are not required.
hub 2017/09/13 20:17:51 Done.
162 }
163
164 void DefaultPlatform::WithTimer(const WithTimerCallback& callback)
165 {
166 std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
167 Platform::WithTimer(callback);
168 }
169
170 void DefaultPlatform::WithFileSystem(const WithFileSystemCallback& callback)
171 {
172 std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
173 Platform::WithFileSystem(callback);
174 }
175
176 void DefaultPlatform::WithWebRequest(const WithWebRequestCallback& callback)
177 {
178 std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
179 Platform::WithWebRequest(callback);
180 }
181
182 void DefaultPlatform::WithLogSystem(const WithLogSystemCallback& callback)
183 {
184 std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
185 Platform::WithLogSystem(callback);
186 }
135 } 187 }
136 188
137 Scheduler DefaultPlatformBuilder::GetDefaultAsyncExecutor() 189 Scheduler DefaultPlatformBuilder::GetDefaultAsyncExecutor()
138 { 190 {
139 if (!defaultScheduler) 191 if (!defaultScheduler)
140 { 192 {
141 asyncExecutor = std::make_shared<Scheduler>(::DummyScheduler); 193 asyncExecutor = std::make_shared<Scheduler>(::DummyScheduler);
142 std::weak_ptr<Scheduler> weakExecutor = asyncExecutor; 194 std::weak_ptr<Scheduler> weakExecutor = asyncExecutor;
143 defaultScheduler = [weakExecutor](const SchedulerTask& task) 195 defaultScheduler = [weakExecutor](const SchedulerTask& task)
144 { 196 {
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
180 if (!timer) 232 if (!timer)
181 CreateDefaultTimer(); 233 CreateDefaultTimer();
182 if (!fileSystem) 234 if (!fileSystem)
183 CreateDefaultFileSystem(); 235 CreateDefaultFileSystem();
184 if (!webRequest) 236 if (!webRequest)
185 CreateDefaultWebRequest(); 237 CreateDefaultWebRequest();
186 238
187 std::unique_ptr<Platform> platform(new DefaultPlatform(asyncExecutor, std::mov e(*this))); 239 std::unique_ptr<Platform> platform(new DefaultPlatform(asyncExecutor, std::mov e(*this)));
188 asyncExecutor.reset(); 240 asyncExecutor.reset();
189 return platform; 241 return platform;
190 } 242 }
OLDNEW
« no previous file with comments | « src/JsEngine.cpp ('k') | src/WebRequestJsObject.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld