Bug fix/cppcheck issues (#10105)

arangod/Agency/Agent.cpp

@@ -58,7 +58,6 @@ std::string const NO_LEADER("");
 /// Agent configuration
 Agent::Agent(ApplicationServer& server, config_t const& config)
     : Thread(server, "Agent"),
-      _server(server),
       _constituent(server),
       _supervision(server),
       _config(config),
@@ -79,9 +78,6 @@ Agent::Agent(ApplicationServer& server, config_t const& config)
   }
 }
 
-/// @brief the underlying application server
-application_features::ApplicationServer& Agent::server() { return _server; }
-
 /// This agent's id
 std::string Agent::id() const { return _config.id(); }
 

arangod/Agency/Agent.h

@@ -51,9 +51,6 @@ class Agent final : public arangodb::Thread, public AgentInterface {
   /// @brief Clean up
   ~Agent();
 
-  /// @brief the underlying application server
-  application_features::ApplicationServer& server();
-
   /// @brief bring down threads, can be called multiple times.
   void waitForThreadsStop();
 
@@ -332,9 +329,6 @@ class Agent final : public arangodb::Thread, public AgentInterface {
   /// @brief Find out, if we've had acknowledged RPCs recent enough
   bool challengeLeadership();
 
-  /// @brief underlying application server
-  application_features::ApplicationServer& _server;
-
   /// @brief Leader election delegate
   Constituent _constituent;
 

arangod/Agency/Constituent.h

@@ -52,7 +52,7 @@ class Agent;
 // RAFT leader election
 class Constituent : public Thread {
  public:
-  Constituent(application_features::ApplicationServer&);
+  explicit Constituent(application_features::ApplicationServer&);
 
   // clean up and exit election
   virtual ~Constituent();

arangod/Agency/Supervision.h

@@ -90,7 +90,7 @@ class Supervision : public arangodb::CriticalThread {
   };
 
   /// @brief Construct sanity checking
-  Supervision(application_features::ApplicationServer& server);
+  explicit Supervision(application_features::ApplicationServer& server);
 
   /// @brief Default dtor
   ~Supervision();

arangod/Aql/BlocksWithClients.cpp

@@ -77,7 +77,9 @@ BlocksWithClients::BlocksWithClients(ExecutionEngine* engine, ExecutionNode cons
 }
 
 std::pair<ExecutionState, bool> BlocksWithClients::getBlock(size_t atMost) {
-  throwIfKilled();  // check if we were aborted
+  if (_engine->getQuery()->killed()) {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED);
+  }
 
   auto res = _dependencies[0]->getSome(atMost);
   if (res.first == ExecutionState::WAITING) {
@@ -127,11 +129,6 @@ size_t BlocksWithClients::getClientId(std::string const& shardId) const {
   return it->second;
 }
 
-void BlocksWithClients::throwIfKilled() {
-  if (_engine->getQuery()->killed()) {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED);
-  }
-}
 std::pair<ExecutionState, SharedAqlItemBlockPtr> BlocksWithClients::getSome(size_t) {
   TRI_ASSERT(false);
   THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);

arangod/Aql/BlocksWithClients.h

@@ -87,9 +87,6 @@ class BlocksWithClients : public ExecutionBlock {
   /// corresponding to <shardId>
   size_t getClientId(std::string const& shardId) const;
 
-  /// @brief throw an exception if query was killed
-  void throwIfKilled();
-
   /// @brief _shardIdMap: map from shardIds to clientNrs
   std::unordered_map<std::string, size_t> _shardIdMap;
 

arangod/Aql/ExpressionContext.h

@@ -64,7 +64,6 @@ class ExpressionContext {
                                                transaction::Methods*,
                                                bool& isEmptyExpression) = 0;
 
-  virtual bool killed() const = 0;
   virtual TRI_vocbase_t& vocbase() const = 0;
   virtual Query* query() const = 0;
 };

arangod/Aql/Functions.cpp

@@ -4186,19 +4186,21 @@ AqlValue Functions::Sleep(ExpressionContext* expressionContext,
     return AqlValue(AqlValueHintNull());
   }
 
-  double now = TRI_microtime();
-  double const until = now + value.toDouble();
   auto& server = application_features::ApplicationServer::server();
 
-  while (now < until) {
+  double const sleepValue = value.toDouble();
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  auto now = std::chrono::steady_clock::now();
+  auto const endTime = now + std::chrono::milliseconds(static_cast<int64_t>(sleepValue * 1000.0));
 
-    if (expressionContext->killed()) {
+  while (now < endTime) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+
+    if (expressionContext->query()->killed()) {
       THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED);
     } else if (server.isStopping()) {
       THROW_ARANGO_EXCEPTION(TRI_ERROR_SHUTTING_DOWN);
     }
-    now = TRI_microtime();
+    now = std::chrono::steady_clock::now();
   }
   return AqlValue(AqlValueHintNull());
 }

arangod/Aql/QueryExpressionContext.cpp

@@ -54,8 +54,6 @@ icu::RegexMatcher* QueryExpressionContext::buildSplitMatcher(AqlValue splitExpre
   return _query->regexCache()->buildSplitMatcher(splitExpression, trx, isEmptyExpression);
 }
 
-bool QueryExpressionContext::killed() const { return _query->killed(); }
-
 TRI_vocbase_t& QueryExpressionContext::vocbase() const {
   return _query->vocbase();
 }

arangod/Aql/QueryExpressionContext.h

@@ -45,7 +45,6 @@ class QueryExpressionContext : public ExpressionContext {
   icu::RegexMatcher* buildSplitMatcher(AqlValue splitExpression, transaction::Methods*,
                                        bool& isEmptyExpression) override;
 
-  bool killed() const override final;
   TRI_vocbase_t& vocbase() const override final;
   Query* query() const override final;
 

arangod/Cluster/ClusterComm.h

@@ -466,7 +466,7 @@ class ClusterComm {
   //////////////////////////////////////////////////////////////////////////////
 
   ClusterComm(application_features::ApplicationServer&);
-  ClusterComm(ClusterComm const&);     // not implemented
+  explicit ClusterComm(ClusterComm const&);     // not implemented
   void operator=(ClusterComm const&);  // not implemented
 
   //////////////////////////////////////////////////////////////////////////////
@@ -717,7 +717,7 @@ class ClusterCommThread : public Thread {
   ClusterCommThread& operator=(ClusterCommThread const&);
 
  public:
-  ClusterCommThread(application_features::ApplicationServer&);
+  explicit ClusterCommThread(application_features::ApplicationServer&);
   ~ClusterCommThread();
  public:
   void beginShutdown() override;

arangod/Pregel/Recovery.h

@@ -52,7 +52,7 @@ class RecoveryManager {
   void _renewPrimaryServer(ShardID const& shard);
 
  public:
-  RecoveryManager(ClusterInfo&);
+  explicit RecoveryManager(ClusterInfo&);
   ~RecoveryManager();
 
   void monitorCollections(DatabaseID const& database,

arangod/Replication/ReplicationApplierConfiguration.h

@@ -81,7 +81,7 @@ class ReplicationApplierConfiguration {
   std::string _clientInfoString;
 
  public:
-  ReplicationApplierConfiguration(application_features::ApplicationServer&);
+  explicit ReplicationApplierConfiguration(application_features::ApplicationServer&);
   ~ReplicationApplierConfiguration() = default;
 
   ReplicationApplierConfiguration(ReplicationApplierConfiguration const&) = default;

arangod/RestServer/DatabaseFeature.h

@@ -57,7 +57,7 @@ class DatabaseManagerThread final : public Thread {
   DatabaseManagerThread(DatabaseManagerThread const&) = delete;
   DatabaseManagerThread& operator=(DatabaseManagerThread const&) = delete;
 
-  DatabaseManagerThread(application_features::ApplicationServer&);
+  explicit DatabaseManagerThread(application_features::ApplicationServer&);
   ~DatabaseManagerThread();
 
   void run() override;

arangod/RocksDBEngine/RocksDBEventListener.h

@@ -79,7 +79,7 @@ protected:
 ////////////////////////////////////////////////////////////////////////////////
 class RocksDBEventListener : public rocksdb::EventListener {
  public:
-  RocksDBEventListener(application_features::ApplicationServer&);
+  explicit RocksDBEventListener(application_features::ApplicationServer&);
   virtual ~RocksDBEventListener();
 
   void OnFlushCompleted(rocksdb::DB* db, const rocksdb::FlushJobInfo& flush_job_info) override;

arangod/Statistics/StatisticsFeature.cpp

@@ -84,7 +84,8 @@ StatisticsDistribution TRI_TotalTimeDistributionStatistics(TRI_RequestTimeDistri
 
 class StatisticsThread final : public Thread {
  public:
-  StatisticsThread(ApplicationServer& server) : Thread(server, "Statistics") {}
+  explicit StatisticsThread(ApplicationServer& server) 
+    : Thread(server, "Statistics") {}
   ~StatisticsThread() { shutdown(); }
 
  public:

arangod/StorageEngine/HotBackup.h

@@ -44,7 +44,7 @@ enum BACKUP_ENGINE {ROCKSDB, MMFILES, CLUSTER};
 
 class HotBackup {
 public:
- HotBackup(application_features::ApplicationServer& server);
+ explicit HotBackup(application_features::ApplicationServer& server);
  virtual ~HotBackup() = default;
 
  /**

lib/Basics/Thread.cpp

@@ -158,13 +158,13 @@ std::string Thread::stringify(ThreadState state) {
 Thread::Thread(application_features::ApplicationServer& server, std::string const& name,
                bool deleteOnExit, std::uint32_t terminationTimeout)
     : _server(server),
-      _deleteOnExit(deleteOnExit),
       _threadStructInitialized(false),
       _refs(0),
       _name(name),
       _thread(),
       _threadNumber(0),
       _terminationTimeout(terminationTimeout),
+      _deleteOnExit(deleteOnExit),
       _finishedCondition(nullptr),
       _state(ThreadState::CREATED) {
   TRI_InitThread(&_thread);

lib/Basics/Thread.h

@@ -89,13 +89,15 @@ class Thread {
          bool deleteOnExit = false, std::uint32_t terminationTimeout = INFINITE);
   virtual ~Thread();
 
- public:
   // whether or not the thread is allowed to start during prepare
   virtual bool isSystem() { return false; }
 
   /// @brief whether or not the thread is chatty on shutdown
   virtual bool isSilent() { return false; }
 
+  /// @brief the underlying application server
+  application_features::ApplicationServer& server() { return _server; }
+
   /// @brief flags the thread as stopping
   /// Classes that override this function must ensure that they
   /// always call Thread::beginShutdown()!
@@ -144,9 +146,6 @@ class Thread {
   /// be threadsafe!
   void shutdown();
 
- protected:
-  application_features::ApplicationServer& _server;
-
  protected:
   /// @brief the thread program
   virtual void run() = 0;
@@ -161,8 +160,10 @@ class Thread {
   void runMe();
   void releaseRef();
  
+ protected:
+  application_features::ApplicationServer& _server;
+
  private:
-  bool const _deleteOnExit;
   std::atomic<bool> _threadStructInitialized;
   std::atomic<int> _refs;
 
@@ -178,6 +179,8 @@ class Thread {
   // The default value is INFINITE, i.e., we want to wait forever instead of aborting the process.
   std::uint32_t _terminationTimeout;
   
+  bool const _deleteOnExit;
+
   basics::ConditionVariable* _finishedCondition;
 
   std::atomic<ThreadState> _state;