Commit 89a26f89 authored by Francis Dupont's avatar Francis Dupont Committed by Tomek Mrugalski

[#880] Addressed comments

parent 88dfca3f
......@@ -87,10 +87,11 @@ increase the time it takes each unit test to execute.
@section unitTestsSanitizers Use Sanitizers
GCC and LLVM support some sanitizers which perform additional tests
at runtime, for instance the ThreadSanitizer (aka TSan) detects
data race in executed C++ code (unfortunately it intercepts signals
and fails to send them to waiting select system calls so some tests
always fail when it is used).
at runtime, for instance the ThreadSanitizer (aka TSan) detects data
race in executed C++ code (unfortunately on macOS it intercepts
signals and fails to send them to waiting select system calls so
some tests always fail when it is used, experiments are run with
different versions of Tsan).
The simplest way to enable a sanitizer is to add it to the CXX
environment variable in .configure by e.g. <i>-fsanitize=thread</i>.
......
......@@ -1032,7 +1032,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
// DHCP packets are waiting so we don't starve external
// sockets under heavy DHCP load.
struct timeval select_timeout;
if (!getPacketQueue4()->canDequeue()) {
if (getPacketQueue4()->empty()) {
select_timeout.tv_sec = timeout_sec;
select_timeout.tv_usec = timeout_usec;
} else {
......@@ -1045,7 +1045,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
if ((result == 0) && !getPacketQueue4()->canDequeue()) {
if ((result == 0) && getPacketQueue4()->empty()) {
// nothing received and timeout has been reached
return (Pkt4Ptr());
} else if (result < 0) {
......@@ -1373,7 +1373,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// DHCP packets are waiting so we don't starve external
// sockets under heavy DHCP load.
struct timeval select_timeout;
if (!getPacketQueue6()->canDequeue()) {
if (getPacketQueue6()->empty()) {
select_timeout.tv_sec = timeout_sec;
select_timeout.tv_usec = timeout_usec;
} else {
......@@ -1386,7 +1386,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
if ((result == 0) && !getPacketQueue6()->canDequeue()) {
if ((result == 0) && getPacketQueue6()->empty()) {
// nothing received and timeout has been reached
return (Pkt6Ptr());
} else if (result < 0) {
......
......@@ -82,11 +82,6 @@ public:
/// @brief return True if the queue is empty.
virtual bool empty() const = 0;
/// @brief return True if dequeue will not return null.
///
/// In fact it is !empty within a lock guard.
virtual bool canDequeue() = 0;
/// @brief Returns the current number of packets in the buffer.
virtual size_t getSize() const = 0;
......
// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -11,6 +11,7 @@
#include <boost/function.hpp>
#include <boost/circular_buffer.hpp>
#include <boost/scoped_ptr.hpp>
#include <sstream>
#include <mutex>
......@@ -36,6 +37,7 @@ public:
PacketQueueRing(const std::string& queue_type, size_t capacity)
: PacketQueue<PacketTypePtr>(queue_type) {
queue_.set_capacity(capacity);
mutex_.reset(new std::mutex);
}
/// @brief virtual Destructor
......@@ -105,7 +107,7 @@ public:
/// @param to specifies the end of the queue to which the packet
/// should be added.
virtual void pushPacket(PacketTypePtr& packet, const QueueEnd& to=QueueEnd::BACK) {
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::mutex> lock(*mutex_);
if (to == QueueEnd::BACK) {
queue_.push_back(packet);
} else {
......@@ -123,9 +125,9 @@ public:
/// @return A pointer to dequeued packet, or an empty pointer
/// if the queue is empty.
virtual PacketTypePtr popPacket(const QueueEnd& from = QueueEnd::FRONT) {
std::lock_guard<std::mutex> lock(mutex_);
PacketTypePtr packet;
std::lock_guard<std::mutex> lock(*mutex_);
if (queue_.empty()) {
return (packet);
}
......@@ -162,14 +164,7 @@ public:
/// @brief Returns True if the queue is empty.
virtual bool empty() const {
return (queue_.empty());
}
/// @brief return True if dequeue will not return null.
/// Use the mutex so race free with a concurrent enqueue.
virtual bool canDequeue() {
std::lock_guard<std::mutex> lock(mutex_);
return (!queue_.empty());
return(queue_.empty());
}
/// @brief Returns the maximum number of packets allowed in the buffer.
......@@ -218,7 +213,7 @@ private:
boost::circular_buffer<PacketTypePtr> queue_;
/// @brief Mutex for protecting queue accesses.
std::mutex mutex_;
boost::scoped_ptr<std::mutex> mutex_;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment