Fix multiple minor issues when running Address Sanitizer (#1345)

lynckia · Jan 29, 2019 · 2eb262c · 2eb262c
1 parent a707296
commit 2eb262c
Show file tree

Hide file tree

Showing 16 changed files with 43 additions and 59 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -106,3 +106,9 @@ jobs:
           name: Create release
           command: |
             ./utils/release.sh -r -v ${RELEASE_VERSION}
+
+workflows:
+  version: 2
+  build:
+    jobs:
+      - build
diff --git a/.travis.yml b/.travis.yml
diff --git a/erizo/src/CMakeLists.txt b/erizo/src/CMakeLists.txt
@@ -116,12 +116,17 @@ if(COMPILE_EXAMPLES)
 endif(COMPILE_EXAMPLES)
 
 ## Tests
+if(${ERIZO_BUILD_TYPE} STREQUAL "sanitizer")
+  set(SANITIZER_OPTION "-DCMAKE_CXX_FLAGS=-fsanitize=address")
+else()
+  set(SANITIZER_OPTION "")
+endif()
 set(GMOCK_BUILD "${CMAKE_CURRENT_BINARY_DIR}/libdeps/gmock")
 set(GMOCK_VERSION "1.8.0")
 ExternalProject_Add(gtest
   URL "https://github.com/google/googletest/archive/release-${GMOCK_VERSION}.tar.gz"
   PREFIX ${GMOCK_BUILD}
-  CMAKE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${GMOCK_BUILD}
+  CMAKE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${GMOCK_BUILD} ${SANITIZER_OPTION}
 )
 
 enable_testing()

diff --git a/erizo/src/erizo/CMakeLists.txt b/erizo/src/erizo/CMakeLists.txt
@@ -7,6 +7,9 @@ set(ERIZO_VERSION_MINOR 1)
 if(${ERIZO_BUILD_TYPE} STREQUAL "debug")
   message("Generating DEBUG project")
   set(CMAKE_CXX_FLAGS "-g -Wall -std=c++11 ${ERIZO_CMAKE_CXX_FLAGS}")
+elseif(${ERIZO_BUILD_TYPE} STREQUAL "sanitizer")
+    message("Generating SANITIZER project")
+    set(CMAKE_CXX_FLAGS "-g -Wall -std=c++11 ${ERIZO_CMAKE_CXX_FLAGS} -O1 -fno-omit-frame-pointer -fsanitize=address -fno-optimize-sibling-calls")
 else()
   message("Generating RELEASE project")
   set(CMAKE_CXX_FLAGS "-g -Wall -O3 -std=c++11 ${ERIZO_CMAKE_CXX_FLAGS}")

diff --git a/erizo/src/erizo/NicerConnection.h b/erizo/src/erizo/NicerConnection.h
@@ -97,7 +97,7 @@ class NicerConnection : public IceConnection, public std::enable_shared_from_thi
   std::shared_ptr<IOWorker> io_worker_;
   std::shared_ptr<NicerInterface> nicer_;
   IceConfig ice_config_;
-  bool closed_;
+  std::atomic<bool> closed_;
   const std::string name_;
   nr_ice_ctx *ctx_;
   nr_ice_peer_ctx *peer_;

diff --git a/erizo/src/erizo/lib/Clock.h b/erizo/src/erizo/lib/Clock.h
@@ -12,6 +12,7 @@ using duration = std::chrono::steady_clock::duration;
 class Clock {
  public:
   virtual time_point now() = 0;
+  virtual ~Clock() {}
 };
 
 class SteadyClock : public Clock {

diff --git a/erizo/src/erizo/lib/LibNiceInterface.h b/erizo/src/erizo/lib/LibNiceInterface.h
@@ -18,6 +18,7 @@ namespace erizo {
 class LibNiceInterface {
  public:
   virtual NiceAgent* NiceAgentNew(GMainContext* context) = 0;
+  virtual ~LibNiceInterface() {}
   virtual char* NiceInterfacesGetIpForInterface(const char *interface_name) = 0;
   virtual int NiceAgentAddStream(NiceAgent* agent, unsigned int n_components) = 0;
   virtual bool NiceAgentGetLocalCredentials(NiceAgent* agent, unsigned int stream_id,

diff --git a/erizo/src/erizo/lib/NicerInterface.h b/erizo/src/erizo/lib/NicerInterface.h
@@ -33,6 +33,7 @@ namespace erizo {
 
 class NicerInterface {
  public:
+  virtual ~NicerInterface() {}
   virtual int IceContextCreate(char *label, UINT4 flags, nr_ice_ctx **ctxp) = 0;
   virtual int IceContextCreateWithCredentials(char *label, UINT4 flags, char* ufrag, char* pwd, nr_ice_ctx **ctxp) = 0;
   virtual int IceContextDestroy(nr_ice_ctx **ctxp) = 0;

diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h
@@ -29,6 +29,7 @@ class RemoteBitrateEstimatorPicker {
  public:
   virtual std::unique_ptr<RemoteBitrateEstimator> pickEstimator(bool using_absolute_send_time,
     webrtc::Clock* const clock, RemoteBitrateObserver *observer);
+  virtual ~RemoteBitrateEstimatorPicker() {}
 };
 
 class BandwidthEstimationHandler: public Handler, public RemoteBitrateObserver,

diff --git a/erizo/src/erizo/rtp/LayerDetectorHandler.cpp b/erizo/src/erizo/rtp/LayerDetectorHandler.cpp
@@ -15,12 +15,12 @@ DEFINE_LOGGER(LayerDetectorHandler, "rtp.LayerDetectorHandler");
 
 LayerDetectorHandler::LayerDetectorHandler(std::shared_ptr<erizo::Clock> the_clock)
     : clock_{the_clock}, stream_{nullptr}, enabled_{true}, initialized_{false},
-    last_event_sent_{clock_->now()} {
-  for (uint32_t temporal_layer = 0; temporal_layer <= kMaxTemporalLayers; temporal_layer++) {
-    video_frame_rate_list_.push_back(MovingIntervalRateStat{std::chrono::milliseconds(500), 10, .5, clock_});
-  }
-  video_frame_width_list_ = std::vector<uint32_t>(kMaxSpatialLayers);
-  video_frame_height_list_ = std::vector<uint32_t>(kMaxSpatialLayers);
+    last_event_sent_{the_clock->now()} {
+  video_ssrc_list_ = std::vector<uint32_t>(kMaxSpatialLayers, 0);
+  video_frame_height_list_ = std::vector<uint32_t>(kMaxSpatialLayers, 0);
+  video_frame_width_list_ = std::vector<uint32_t>(kMaxSpatialLayers, 0);
+  video_frame_rate_list_ = std::vector<MovingIntervalRateStat>(kMaxTemporalLayers,
+      MovingIntervalRateStat{std::chrono::milliseconds(500), 10, .5, clock_});
 }
 
 void LayerDetectorHandler::enable() {

diff --git a/erizo/src/erizo/stats/StatNode.h b/erizo/src/erizo/stats/StatNode.h
@@ -43,7 +43,6 @@ class StatNode {
   virtual std::string toString();
 
  private:
-  bool is_node_{false};
   std::map<std::string, std::shared_ptr<StatNode>> node_map_;
 };
 

diff --git a/erizo/src/erizo/thread/IOWorker.h b/erizo/src/erizo/thread/IOWorker.h
@@ -14,7 +14,7 @@ class IOWorker : public std::enable_shared_from_this<IOWorker> {
  public:
   typedef std::function<void()> Task;
   IOWorker();
-  ~IOWorker();
+  virtual ~IOWorker();
 
   virtual void start();
   virtual void start(std::shared_ptr<std::promise<void>> start_promise);

diff --git a/erizo/src/erizo/thread/Scheduler.cpp b/erizo/src/erizo/thread/Scheduler.cpp
@@ -20,6 +20,10 @@ Scheduler::~Scheduler() {
   assert(n_threads_servicing_queue_ == 0);
 }
 
+std::chrono::system_clock::time_point Scheduler::getFirstTime() {
+  return task_queue_.empty() ? std::chrono::system_clock::now() : task_queue_.begin()->first;
+}
+
 void Scheduler::serviceQueue() {
   std::unique_lock<std::mutex> lock(new_task_mutex_);
 
@@ -29,8 +33,10 @@ void Scheduler::serviceQueue() {
         new_task_scheduled_.wait(lock);
       }
 
+      std::chrono::system_clock::time_point time = getFirstTime();
       while (!stop_requested_ && !task_queue_.empty() &&
-             new_task_scheduled_.wait_until(lock, task_queue_.begin()->first) != std::cv_status::timeout) {
+             new_task_scheduled_.wait_until(lock, time) != std::cv_status::timeout) {
+        time = getFirstTime();
       }
       if (stop_requested_) {
         break;

diff --git a/erizo/src/erizo/thread/Scheduler.h b/erizo/src/erizo/thread/Scheduler.h
@@ -34,6 +34,7 @@ class Scheduler {
 
  private:
   void serviceQueue();
+  std::chrono::system_clock::time_point getFirstTime();
 
  private:
   std::multimap<std::chrono::system_clock::time_point, Function> task_queue_;

diff --git a/erizo/src/erizo/thread/Worker.h b/erizo/src/erizo/thread/Worker.h
@@ -34,7 +34,7 @@ class Worker : public std::enable_shared_from_this<Worker> {
 
   explicit Worker(std::weak_ptr<Scheduler> scheduler,
                   std::shared_ptr<Clock> the_clock = std::make_shared<SteadyClock>());
-  ~Worker();
+  virtual ~Worker();
 
   virtual void task(Task f);
 

diff --git a/erizo/src/test/CMakeLists.txt b/erizo/src/test/CMakeLists.txt
@@ -2,7 +2,13 @@ cmake_minimum_required(VERSION 2.6)
 
 project (ERIZO_TEST)
 
-set(CMAKE_CXX_FLAGS "-g -Wall -std=c++11 ${ERIZO_CMAKE_CXX_FLAGS}")
+if(${ERIZO_BUILD_TYPE} STREQUAL "sanitizer")
+  set(SANITIZER_OPTION "-fsanitize=address")
+else()
+  set(SANITIZER_OPTION "")
+endif()
+
+set(CMAKE_CXX_FLAGS "-g -Wall -std=c++11 ${ERIZO_CMAKE_CXX_FLAGS} ${SANITIZER_OPTION}")
 
 file(GLOB_RECURSE ERIZO_TEST_SDPS ${ERIZO_TEST_SOURCE_DIR}/*.sdp)
 file(COPY ${ERIZO_TEST_SDPS} DESTINATION ${ERIZO_TEST_BINARY_DIR})