Issue 5118 - Lock the platform interfaces before use

src/Platform.cpp

 /*
 * This file is part of Adblock Plus <https://adblockplus.org/>,
 * Copyright (C) 2006-present eyeo GmbH
 *
 * Adblock Plus is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 3 as
 * published by the Free Software Foundation.
 *
 * Adblock Plus is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 113 matching lines @@
   class DefaultPlatform : public Platform
   {
   public:
     typedef std::shared_ptr<Scheduler> AsyncExecutorPtr;
     explicit DefaultPlatform(const AsyncExecutorPtr& asyncExecutor, CreationParameters&& creationParams)
       : Platform(std::move(creationParams)), asyncExecutor(asyncExecutor)
     {
     }
     ~DefaultPlatform();
 
-    virtual void WithTimer(const WithTimerCallback&) override;

[sergei, 2017/09/13 20:06:37]

Could you please remove the `virtual` keyword here for overridden methods
because it's already clear that they are virtual from `override`.

[hub, 2017/09/13 20:17:51]

Done.

-    virtual void WithFileSystem(const WithFileSystemCallback&) override;
-    virtual void WithWebRequest(const WithWebRequestCallback&) override;
-    virtual void WithLogSystem(const WithLogSystemCallback&) override;
+    void WithTimer(const WithTimerCallback&) override;
+    void WithFileSystem(const WithFileSystemCallback&) override;
+    void WithWebRequest(const WithWebRequestCallback&) override;
+    void WithLogSystem(const WithLogSystemCallback&) override;
 
   private:
     AsyncExecutorPtr asyncExecutor;
     std::recursive_mutex interfacesMutex;
   };
 
   DefaultPlatform::~DefaultPlatform()
   {
     asyncExecutor.reset();
     LogSystemPtr tmpLogSystem;
     TimerPtr tmpTimer;
     FileSystemPtr tmpFileSystem;
     WebRequestPtr tmpWebRequest;
     {
       std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
       tmpLogSystem = std::move(logSystem);
       tmpTimer = std::move(timer);
       tmpFileSystem = std::move(fileSystem);
       tmpWebRequest = std::move(webRequest);
     }
-    tmpLogSystem.reset();
-    tmpTimer.reset();
-    tmpFileSystem.reset();
-    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.

   }
 
   void DefaultPlatform::WithTimer(const WithTimerCallback& callback)
   {
     std::lock_guard<std::recursive_mutex> lock(interfacesMutex);
     Platform::WithTimer(callback);
   }
 
   void DefaultPlatform::WithFileSystem(const WithFileSystemCallback& callback)
   {
@@ skipping 61 matching lines @@
     CreateDefaultTimer();
   if (!fileSystem)
     CreateDefaultFileSystem();
   if (!webRequest)
     CreateDefaultWebRequest();
 
   std::unique_ptr<Platform> platform(new DefaultPlatform(asyncExecutor, std::move(*this)));
   asyncExecutor.reset();
   return platform;
 }