Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple minor issues when running Address Sanitizer #1345

Merged
merged 10 commits into from
Jan 29, 2019
Merged
6 changes: 6 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,9 @@ jobs:
name: Create release
command: |
./utils/release.sh -r -v ${RELEASE_VERSION}

workflows:
version: 2
build:
jobs:
- build
7 changes: 6 additions & 1 deletion erizo/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,17 @@ if(COMPILE_EXAMPLES)
endif(COMPILE_EXAMPLES)

## Tests
if(${ERIZO_BUILD_TYPE} STREQUAL "sanitizer")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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()
Expand Down
3 changes: 3 additions & 0 deletions erizo/src/erizo/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
2 changes: 1 addition & 1 deletion erizo/src/erizo/NicerConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/lib/Clock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/lib/LibNiceInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/lib/NicerInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/rtp/BandwidthEstimationHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions erizo/src/erizo/rtp/LayerDetectorHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion erizo/src/erizo/stats/StatNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
};

Expand Down
2 changes: 1 addition & 1 deletion erizo/src/erizo/thread/IOWorker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion erizo/src/erizo/thread/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_);

Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/thread/Scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
2 changes: 1 addition & 1 deletion erizo/src/erizo/thread/Worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
8 changes: 7 additions & 1 deletion erizo/src/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down