From 2eb262ca46d9191ad093b0cdd3c076fbfc4806dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Cervi=C3=B1o?= Date: Tue, 29 Jan 2019 18:03:23 +0100 Subject: [PATCH] Fix multiple minor issues when running Address Sanitizer (#1345) --- .circleci/config.yml | 6 +++ .travis.yml | 46 ------------------- erizo/src/CMakeLists.txt | 7 ++- erizo/src/erizo/CMakeLists.txt | 3 ++ erizo/src/erizo/NicerConnection.h | 2 +- erizo/src/erizo/lib/Clock.h | 1 + erizo/src/erizo/lib/LibNiceInterface.h | 1 + erizo/src/erizo/lib/NicerInterface.h | 1 + .../erizo/rtp/BandwidthEstimationHandler.h | 1 + erizo/src/erizo/rtp/LayerDetectorHandler.cpp | 12 ++--- erizo/src/erizo/stats/StatNode.h | 1 - erizo/src/erizo/thread/IOWorker.h | 2 +- erizo/src/erizo/thread/Scheduler.cpp | 8 +++- erizo/src/erizo/thread/Scheduler.h | 1 + erizo/src/erizo/thread/Worker.h | 2 +- erizo/src/test/CMakeLists.txt | 8 +++- 16 files changed, 43 insertions(+), 59 deletions(-) delete mode 100644 .travis.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 3a4725f132..f95133f3ca 100644 --- 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 deleted file mode 100644 index 1e2d89c5e2..0000000000 --- a/.travis.yml +++ /dev/null @@ -1,46 +0,0 @@ -language: node_js -os: -# - osx TODO Enable again once we add openssl, libnice, libsrtp and libav to cmake - - linux -sudo: required -dist: trusty -cache: - directories: - - node_modules - - cache - - build/libdeps -services: - - mongodb - - rabbitmq -addons: - apt: - packages: - - git - - make - - gcc-5 - - g++-5 - - libssl-dev - - cmake - - libglib2.0-dev - - pkg-config - - libboost-regex-dev - - libboost-thread-dev - - libboost-system-dev - - liblog4cxx10-dev - - curl - - libboost-test-dev - - yasm - - libvpx. - - libx264. - sources: - - ubuntu-toolchain-r-test -before_install: - # OSX - - if [ "$TRAVIS_OS_NAME" == "osx" ]; then ./scripts/installMacDeps.sh --unattended --disable-services --use-cache; fi - - # LINUX - - if [ "$TRAVIS_OS_NAME" == "linux" ]; then ./scripts/travisInstallDeps.sh && export CC="/usr/bin/gcc-5" && export CXX="/usr/bin/g++-5" ; fi -install: - - ./scripts/travisInstall.sh -script: - - ./scripts/travisScript.sh diff --git a/erizo/src/CMakeLists.txt b/erizo/src/CMakeLists.txt index 51023b012a..c3b3495ba2 100644 --- 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 index 8b5d18b5f9..320641c4ff 100644 --- 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 index bc7cecd4b0..e467358826 100644 --- 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 io_worker_; std::shared_ptr nicer_; IceConfig ice_config_; - bool closed_; + std::atomic 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 index 108ad7ce83..49801f0f8e 100644 --- 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 index dd47b76b08..cfd9d06e7c 100644 --- 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 index 3fa0975aa7..ed1700ba33 100644 --- 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 index c851ab1d97..7582a8f336 100644 --- 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 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 index 2e00e603bd..24ecd01fe8 100644 --- 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 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(kMaxSpatialLayers); - video_frame_height_list_ = std::vector(kMaxSpatialLayers); + last_event_sent_{the_clock->now()} { + video_ssrc_list_ = std::vector(kMaxSpatialLayers, 0); + video_frame_height_list_ = std::vector(kMaxSpatialLayers, 0); + video_frame_width_list_ = std::vector(kMaxSpatialLayers, 0); + video_frame_rate_list_ = std::vector(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 index c46173e214..1b718f57ba 100644 --- 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> node_map_; }; diff --git a/erizo/src/erizo/thread/IOWorker.h b/erizo/src/erizo/thread/IOWorker.h index 93a7b4f4e0..14f2fbad7e 100644 --- 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 { public: typedef std::function Task; IOWorker(); - ~IOWorker(); + virtual ~IOWorker(); virtual void start(); virtual void start(std::shared_ptr> start_promise); diff --git a/erizo/src/erizo/thread/Scheduler.cpp b/erizo/src/erizo/thread/Scheduler.cpp index a5fba22fdd..1c90ec24b2 100644 --- 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 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 index d62c44b79a..3a4120874a 100644 --- 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 task_queue_; diff --git a/erizo/src/erizo/thread/Worker.h b/erizo/src/erizo/thread/Worker.h index 58ca33673f..34abb77717 100644 --- 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 { explicit Worker(std::weak_ptr scheduler, std::shared_ptr the_clock = std::make_shared()); - ~Worker(); + virtual ~Worker(); virtual void task(Task f); diff --git a/erizo/src/test/CMakeLists.txt b/erizo/src/test/CMakeLists.txt index f11aab55f5..3c3c73a4df 100644 --- 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})