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

Issue 29499583: Issue 4938 - fix race conditions related to LazyFileSystem (Closed)

Created:
July 27, 2017, 8:53 a.m. by sergei
Modified:
July 27, 2017, 12:48 p.m.
Reviewers:
hub
CC:
Felix Dahlke
Base URL:
https://github.com/adblockplus/libadblockplus.git
Visibility:
Public.

Description

The main point here is in the adding of the scheduler dependency to LazyFileSystem which by default executes tasks immediately. It allows to control the execution of file system tasks by accumulating of pending ones and executing them one by one until the desired state is reached as well as check the state between tasks. It also gets rid of any threads what simplifies the debugging and eliminates a possibility for race conditions. In order to simplify the creation of FilterEngine using custom scheduler there is a helper method `FilterEnginePtr CreateFilterEngine(LazyFileSystem& fileSystem, const JsEnginePtr& jsEngine, const FilterEngine::CreationParameters& creationParams = FilterEngine::CreationParameters());`. All places where it's actually used are accordingly changed.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+126 lines, -103 lines) Patch
M test/BaseJsTest.h View 1 chunk +42 lines, -57 lines 0 comments Download
M test/BaseJsTest.cpp View 2 chunks +26 lines, -0 lines 0 comments Download
M test/FilterEngine.cpp View 6 chunks +14 lines, -15 lines 2 comments Download
M test/Notification.cpp View 1 chunk +3 lines, -2 lines 0 comments Download
M test/Prefs.cpp View 4 chunks +33 lines, -23 lines 0 comments Download
M test/UpdateCheck.cpp View 2 chunks +3 lines, -2 lines 0 comments Download
M test/WebRequest.cpp View 5 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 2
sergei
https://codereview.adblockplus.org/29499583/diff/29499584/test/FilterEngine.cpp File test/FilterEngine.cpp (left): https://codereview.adblockplus.org/29499583/diff/29499584/test/FilterEngine.cpp#oldcode65 test/FilterEngine.cpp:65: std::this_thread::sleep_for(std::chrono::milliseconds(100)); Since there is no class waiting for its ...
July 27, 2017, 9:15 a.m. (2017-07-27 09:15:32 UTC) #1
hub
July 27, 2017, 12:42 p.m. (2017-07-27 12:42:02 UTC) #2
LGTM

Powered by Google App Engine
This is Rietveld