encoding+util: ensure move constructors are properly declared
This also fixes a race condition in scheduler::EventId::operator bool()
Change-Id: I468f0c46039a3d1a38c69c419ae45b4445d8205a
Refs: #3414
diff --git a/src/util/scheduler-scoped-event-id.cpp b/src/util/scheduler-scoped-event-id.cpp
index 0ca7d13..d83fa4c 100644
--- a/src/util/scheduler-scoped-event-id.cpp
+++ b/src/util/scheduler-scoped-event-id.cpp
@@ -25,34 +25,24 @@
namespace util {
namespace scheduler {
-static_assert(std::is_nothrow_move_constructible<ScopedEventId>::value,
- "ScopedEventId must be MoveConstructible with noexcept");
-
-ScopedEventId::ScopedEventId(Scheduler& scheduler)
+ScopedEventId::ScopedEventId(Scheduler& scheduler) noexcept
: m_scheduler(&scheduler)
{
}
-ScopedEventId::ScopedEventId(ScopedEventId&& other) noexcept
- : m_scheduler(other.m_scheduler)
- , m_event(other.m_event)
-{
- other.release();
-}
-
ScopedEventId&
-ScopedEventId::operator=(const EventId& event)
+ScopedEventId::operator=(EventId event)
{
if (m_event != event) {
- m_scheduler->cancelEvent(m_event);
- m_event = event;
+ cancel();
+ m_event = std::move(event);
}
return *this;
}
-ScopedEventId::~ScopedEventId() noexcept
+ScopedEventId::~ScopedEventId()
{
- m_scheduler->cancelEvent(m_event);
+ cancel();
}
void
diff --git a/src/util/scheduler-scoped-event-id.hpp b/src/util/scheduler-scoped-event-id.hpp
index ea27016..703a745 100644
--- a/src/util/scheduler-scoped-event-id.hpp
+++ b/src/util/scheduler-scoped-event-id.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2016 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -28,44 +28,53 @@
namespace util {
namespace scheduler {
-/** \brief Event that is automatically cancelled upon destruction
+/** \brief Event that is automatically cancelled upon destruction.
*/
-class ScopedEventId : noncopyable
+class ScopedEventId
{
public:
- /** \brief Construct ScopedEventId tied to the specified scheduler
+ /** \brief Construct ScopedEventId tied to the specified scheduler.
* \param scheduler Scheduler to which the event is tied. Behavior is undefined if
- * scheduler is destructed before an uncanceled ScopedEventId.
+ * \p scheduler is destructed before an uncanceled ScopedEventId.
*/
explicit
- ScopedEventId(Scheduler& scheduler);
+ ScopedEventId(Scheduler& scheduler) noexcept;
- /** \brief move constructor
+ ScopedEventId(const ScopedEventId&) = delete;
+
+ ScopedEventId&
+ operator=(const ScopedEventId&) = delete;
+
+ /** \brief Move constructor.
*/
- ScopedEventId(ScopedEventId&& other) noexcept;
+ ScopedEventId(ScopedEventId&&) noexcept;
- /** \brief assigns an event
+ /** \brief Move assignment operator.
+ */
+ ScopedEventId&
+ operator=(ScopedEventId&&) noexcept;
+
+ /** \brief Assign an event.
*
* If a different event has been assigned to this instance previously,
* that event will be cancelled immediately.
*
- * \note The caller should ensure that ScopedEventId is tied to a correct scheduler.
- * Behavior is undefined when assigning event scheduled in another scheduler instance.
+ * \note The caller should ensure that this ScopedEventId is tied to the correct Scheduler.
+ * Behavior is undefined when assigning an event scheduled in another Scheduler instance.
*/
ScopedEventId&
- operator=(const EventId& event);
+ operator=(EventId event);
- /** \brief cancels the event
+ /** \brief Destructor, automatically cancels the event.
*/
- ~ScopedEventId() noexcept;
+ ~ScopedEventId();
- /** \brief cancels the event manually
+ /** \brief Manually cancel the event.
*/
void
cancel();
- /** \brief releases the event so that it won't be canceled
- * when this ScopedEventId is destructed
+ /** \brief Release the event so that it won't be canceled when this ScopedEventId is destructed.
*/
void
release() noexcept;
@@ -75,6 +84,12 @@
EventId m_event;
};
+inline
+ScopedEventId::ScopedEventId(ScopedEventId&&) noexcept = default;
+
+inline ScopedEventId&
+ScopedEventId::operator=(ScopedEventId&&) noexcept = default;
+
} // namespace scheduler
} // namespace util
} // namespace ndn
diff --git a/src/util/scheduler.cpp b/src/util/scheduler.cpp
index aecf15a..9e52b25 100644
--- a/src/util/scheduler.cpp
+++ b/src/util/scheduler.cpp
@@ -51,15 +51,16 @@
EventQueue::const_iterator queueIt;
};
-EventId::operator bool() const
+EventId::operator bool() const noexcept
{
- return !m_info.expired() && !m_info.lock()->isExpired;
+ auto sp = m_info.lock();
+ return sp != nullptr && !sp->isExpired;
}
bool
-EventId::operator==(const EventId& other) const
+EventId::operator==(const EventId& other) const noexcept
{
- return (!(*this) && !other) ||
+ return (!*this && !other) ||
!(m_info.owner_before(other.m_info) || other.m_info.owner_before(m_info));
}
@@ -70,7 +71,7 @@
}
bool
-EventQueueCompare::operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const
+EventQueueCompare::operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const noexcept
{
return a->expireTime < b->expireTime;
}
diff --git a/src/util/scheduler.hpp b/src/util/scheduler.hpp
index 79fbf23..bd4467a 100644
--- a/src/util/scheduler.hpp
+++ b/src/util/scheduler.hpp
@@ -57,7 +57,8 @@
* \brief Constructs an empty EventId
* \note EventId is implicitly convertible from nullptr.
*/
- EventId(std::nullptr_t = nullptr)
+ constexpr
+ EventId(std::nullptr_t = nullptr) noexcept
{
}
@@ -66,16 +67,16 @@
* \retval false This EventId is empty, or the event is expired or cancelled.
*/
explicit
- operator bool() const;
+ operator bool() const noexcept;
/**
* \return whether this and other refer to the same event, or are both empty/expired/cancelled
*/
bool
- operator==(const EventId& other) const;
+ operator==(const EventId& other) const noexcept;
bool
- operator!=(const EventId& other) const
+ operator!=(const EventId& other) const noexcept
{
return !this->operator==(other);
}
@@ -86,15 +87,15 @@
* \post !(*this)
*/
void
- reset()
+ reset() noexcept
{
m_info.reset();
}
private:
explicit
- EventId(const weak_ptr<EventInfo>& info)
- : m_info(info)
+ EventId(weak_ptr<EventInfo> info) noexcept
+ : m_info(std::move(info))
{
}
@@ -112,7 +113,7 @@
{
public:
bool
- operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const;
+ operator()(const shared_ptr<EventInfo>& a, const shared_ptr<EventInfo>& b) const noexcept;
};
using EventQueue = std::multiset<shared_ptr<EventInfo>, EventQueueCompare>;
diff --git a/src/util/signal/connection.cpp b/src/util/signal/connection.cpp
index 7d46eb1..1d93326 100644
--- a/src/util/signal/connection.cpp
+++ b/src/util/signal/connection.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -27,26 +27,22 @@
BOOST_CONCEPT_ASSERT((boost::EqualityComparable<Connection>));
-Connection::Connection()
-{
-}
-
-Connection::Connection(weak_ptr<function<void()>> disconnect)
- : m_disconnect(disconnect)
+Connection::Connection(weak_ptr<function<void()>> disconnect) noexcept
+ : m_disconnect(std::move(disconnect))
{
}
void
Connection::disconnect()
{
- shared_ptr<function<void()>> f = m_disconnect.lock();
+ auto f = m_disconnect.lock();
if (f != nullptr) {
(*f)();
}
}
bool
-Connection::isConnected() const
+Connection::isConnected() const noexcept
{
return !m_disconnect.expired();
}
@@ -54,8 +50,8 @@
bool
Connection::operator==(const Connection& other) const
{
- shared_ptr<function<void()>> f1 = m_disconnect.lock();
- shared_ptr<function<void()>> f2 = other.m_disconnect.lock();
+ auto f1 = m_disconnect.lock();
+ auto f2 = other.m_disconnect.lock();
return f1 == f2;
}
diff --git a/src/util/signal/connection.hpp b/src/util/signal/connection.hpp
index 6ee8e1c..528dcba 100644
--- a/src/util/signal/connection.hpp
+++ b/src/util/signal/connection.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -34,7 +34,8 @@
class Connection
{
public:
- Connection();
+ constexpr
+ Connection() noexcept = default;
/** \brief disconnects from the signal
* \note If the connection is already disconnected, or if the Signal has been destructed,
@@ -49,7 +50,7 @@
* \return false if disconnected from the signal
*/
bool
- isConnected() const;
+ isConnected() const noexcept;
/** \brief compare for equality
*
@@ -66,7 +67,7 @@
/** \param disconnect weak_ptr to a function that disconnects the handler
*/
explicit
- Connection(weak_ptr<function<void()>> disconnect);
+ Connection(weak_ptr<function<void()>> disconnect) noexcept;
template<typename Owner, typename ...TArgs>
friend class Signal;
diff --git a/src/util/signal/scoped-connection.cpp b/src/util/signal/scoped-connection.cpp
index 3fed4a3..7a231b0 100644
--- a/src/util/signal/scoped-connection.cpp
+++ b/src/util/signal/scoped-connection.cpp
@@ -25,35 +25,24 @@
namespace util {
namespace signal {
-static_assert(std::is_nothrow_move_constructible<ScopedConnection>::value,
- "ScopedConnection must be MoveConstructible with noexcept");
-
-ScopedConnection::ScopedConnection() = default;
-
-ScopedConnection::ScopedConnection(const Connection& connection)
- : m_connection(connection)
+ScopedConnection::ScopedConnection(Connection connection) noexcept
+ : m_connection(std::move(connection))
{
}
-ScopedConnection::ScopedConnection(ScopedConnection&& other) noexcept
- : m_connection(other.m_connection)
-{
- other.release();
-}
-
ScopedConnection&
-ScopedConnection::operator=(const Connection& connection)
+ScopedConnection::operator=(Connection connection)
{
if (m_connection != connection) {
- m_connection.disconnect();
- m_connection = connection;
+ disconnect();
+ m_connection = std::move(connection);
}
return *this;
}
-ScopedConnection::~ScopedConnection() noexcept
+ScopedConnection::~ScopedConnection()
{
- m_connection.disconnect();
+ disconnect();
}
void
@@ -63,13 +52,13 @@
}
bool
-ScopedConnection::isConnected() const
+ScopedConnection::isConnected() const noexcept
{
return m_connection.isConnected();
}
void
-ScopedConnection::release()
+ScopedConnection::release() noexcept
{
m_connection = {};
}
diff --git a/src/util/signal/scoped-connection.hpp b/src/util/signal/scoped-connection.hpp
index 5c77c2f..f352be0 100644
--- a/src/util/signal/scoped-connection.hpp
+++ b/src/util/signal/scoped-connection.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -28,56 +28,73 @@
namespace util {
namespace signal {
-/** \brief disconnects a Connection automatically upon destruction
+/** \brief Disconnects a Connection automatically upon destruction.
*/
-class ScopedConnection : noncopyable
+class ScopedConnection
{
public:
- ScopedConnection();
+ constexpr
+ ScopedConnection() noexcept = default;
- /** \brief implicit constructor from Connection
+ ScopedConnection(const ScopedConnection&) = delete;
+
+ ScopedConnection&
+ operator=(const ScopedConnection&) = delete;
+
+ /** \brief Move constructor.
+ */
+ ScopedConnection(ScopedConnection&&) noexcept;
+
+ /** \brief Move assignment operator.
+ */
+ ScopedConnection&
+ operator=(ScopedConnection&&) noexcept;
+
+ /** \brief Implicit constructor from Connection.
* \param connection the Connection to be disconnected upon destruction
*/
- ScopedConnection(const Connection& connection);
+ ScopedConnection(Connection connection) noexcept;
- /** \brief move constructor
- */
- ScopedConnection(ScopedConnection&& other) noexcept;
-
- /** \brief assigns a connection
+ /** \brief Assign a connection.
*
* If a different connection has been assigned to this instance previously,
* that connection will be disconnected immediately.
*/
ScopedConnection&
- operator=(const Connection& connection);
+ operator=(Connection connection);
- /** \brief disconnects the connection
+ /** \brief Destructor, automatically disconnects the connection.
*/
- ~ScopedConnection() noexcept;
+ ~ScopedConnection();
- /** \brief disconnects the connection manually
+ /** \brief Manually disconnect the connection.
*/
void
disconnect();
- /** \brief check if the connection is connected to the signal
+ /** \brief Check if the connection is connected to the signal.
* \return false when a default-constructed connection is used, the connection is released,
* or the connection is disconnected
*/
bool
- isConnected() const;
+ isConnected() const noexcept;
- /** \brief releases the connection so that it won't be disconnected
- * when this ScopedConnection is destructed
+ /** \brief Release the connection so that it won't be disconnected
+ * when this ScopedConnection is destructed.
*/
void
- release();
+ release() noexcept;
private:
Connection m_connection;
};
+inline
+ScopedConnection::ScopedConnection(ScopedConnection&&) noexcept = default;
+
+inline ScopedConnection&
+ScopedConnection::operator=(ScopedConnection&&) noexcept = default;
+
} // namespace signal
} // namespace util
} // namespace ndn