Commit 48866d26 authored by Eric Niebler's avatar Eric Niebler Committed by Facebook GitHub Bot

make Executor::keepAlive(Acquire|Release) and KeepAlive move assign noexcept

Summary:
Moveable types should have no-throw move assignment operators. `Executor::KeepAlive<>`'s move assignment operator is not marked `noexcept`. It calls the virtual `Executor::keepAliveRelease`, which is also not marked `noexcept`, despite the fact that virtually (haha) all overrides of that function do nothing more than decrement an atomic and free some resources.

This diff makes the following `noexcept`:
* `Executor::keepAliveAcquire`
* `Executor::keepAliveRelease`
* `KeepAlive::reset()`
* `KeepAlive::operator=(KeepAlive&&)`

 ---

Reviewed By: yfeldblum

Differential Revision: D22505764

fbshipit-source-id: 8e0a04e057c971673cf75da974c1abca2bdf87e8
parent 2f0542e4
...@@ -88,7 +88,7 @@ class DefaultKeepAliveExecutor : public virtual Executor { ...@@ -88,7 +88,7 @@ class DefaultKeepAliveExecutor : public virtual Executor {
executor_(executor), executor_(executor),
numPriorities_(executor->getNumPriorities()) {} numPriorities_(executor->getNumPriorities()) {}
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCount = auto keepAliveCount =
keepAliveCount_.fetch_add(1, std::memory_order_relaxed); keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
// We should never increment from 0 // We should never increment from 0
...@@ -96,7 +96,7 @@ class DefaultKeepAliveExecutor : public virtual Executor { ...@@ -96,7 +96,7 @@ class DefaultKeepAliveExecutor : public virtual Executor {
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCount = auto keepAliveCount =
keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(keepAliveCount >= 1); DCHECK(keepAliveCount >= 1);
...@@ -130,7 +130,7 @@ class DefaultKeepAliveExecutor : public virtual Executor { ...@@ -130,7 +130,7 @@ class DefaultKeepAliveExecutor : public virtual Executor {
uint8_t numPriorities_; uint8_t numPriorities_;
}; };
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCount = auto keepAliveCount =
controlBlock_->keepAliveCount_.fetch_add(1, std::memory_order_relaxed); controlBlock_->keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
// We should never increment from 0 // We should never increment from 0
...@@ -138,7 +138,7 @@ class DefaultKeepAliveExecutor : public virtual Executor { ...@@ -138,7 +138,7 @@ class DefaultKeepAliveExecutor : public virtual Executor {
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCount = auto keepAliveCount =
controlBlock_->keepAliveCount_.fetch_sub(1, std::memory_order_acquire); controlBlock_->keepAliveCount_.fetch_sub(1, std::memory_order_acquire);
DCHECK(keepAliveCount >= 1); DCHECK(keepAliveCount >= 1);
......
...@@ -30,11 +30,11 @@ void Executor::addWithPriority(Func, int8_t /* priority */) { ...@@ -30,11 +30,11 @@ void Executor::addWithPriority(Func, int8_t /* priority */) {
"addWithPriority() is not implemented for this Executor"); "addWithPriority() is not implemented for this Executor");
} }
bool Executor::keepAliveAcquire() { bool Executor::keepAliveAcquire() noexcept {
return false; return false;
} }
void Executor::keepAliveRelease() { void Executor::keepAliveRelease() noexcept {
LOG(FATAL) << __func__ << "() should not be called for folly::Executor types " LOG(FATAL) << __func__ << "() should not be called for folly::Executor types "
<< "which do not override keepAliveAcquire()"; << "which do not override keepAliveAcquire()";
} }
......
...@@ -126,7 +126,7 @@ class Executor { ...@@ -126,7 +126,7 @@ class Executor {
*this = getKeepAliveToken(executor); *this = getKeepAliveToken(executor);
} }
KeepAlive& operator=(KeepAlive&& other) { KeepAlive& operator=(KeepAlive&& other) noexcept {
reset(); reset();
storage_ = std::exchange(other.storage_, 0); storage_ = std::exchange(other.storage_, 0);
return *this; return *this;
...@@ -140,7 +140,7 @@ class Executor { ...@@ -140,7 +140,7 @@ class Executor {
typename OtherExecutor, typename OtherExecutor,
typename = typename std::enable_if< typename = typename std::enable_if<
std::is_convertible<OtherExecutor*, ExecutorT*>::value>::type> std::is_convertible<OtherExecutor*, ExecutorT*>::value>::type>
KeepAlive& operator=(KeepAlive<OtherExecutor>&& other) { KeepAlive& operator=(KeepAlive<OtherExecutor>&& other) noexcept {
return *this = KeepAlive(std::move(other)); return *this = KeepAlive(std::move(other));
} }
...@@ -152,7 +152,7 @@ class Executor { ...@@ -152,7 +152,7 @@ class Executor {
return *this = KeepAlive(other); return *this = KeepAlive(other);
} }
void reset() { void reset() noexcept {
if (Executor* executor = get()) { if (Executor* executor = get()) {
auto const flags = std::exchange(storage_, 0) & kFlagMask; auto const flags = std::exchange(storage_, 0) & kFlagMask;
if (!(flags & (kDummyFlag | kAliasFlag))) { if (!(flags & (kDummyFlag | kAliasFlag))) {
...@@ -251,10 +251,10 @@ class Executor { ...@@ -251,10 +251,10 @@ class Executor {
// Acquire a keep alive token. Should return false if keep-alive mechanism // Acquire a keep alive token. Should return false if keep-alive mechanism
// is not supported. // is not supported.
virtual bool keepAliveAcquire(); virtual bool keepAliveAcquire() noexcept;
// Release a keep alive token previously acquired by keepAliveAcquire(). // Release a keep alive token previously acquired by keepAliveAcquire().
// Will never be called if keepAliveAcquire() returns false. // Will never be called if keepAliveAcquire() returns false.
virtual void keepAliveRelease(); virtual void keepAliveRelease() noexcept;
template <typename ExecutorT> template <typename ExecutorT>
static KeepAlive<ExecutorT> makeKeepAlive(ExecutorT* executor) { static KeepAlive<ExecutorT> makeKeepAlive(ExecutorT* executor) {
......
...@@ -311,14 +311,14 @@ folly::Executor::KeepAlive<> EDFThreadPoolExecutor::deadlineExecutor( ...@@ -311,14 +311,14 @@ folly::Executor::KeepAlive<> EDFThreadPoolExecutor::deadlineExecutor(
executor_->add(std::move(f), deadline_); executor_->add(std::move(f), deadline_);
} }
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
const auto count = const auto count =
keepAliveCount_.fetch_add(1, std::memory_order_relaxed); keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
DCHECK_GT(count, 0); DCHECK_GT(count, 0);
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
const auto count = const auto count =
keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK_GT(count, 0); DCHECK_GT(count, 0);
......
...@@ -37,14 +37,14 @@ class ExecutorWithPriorityImpl : public virtual Executor { ...@@ -37,14 +37,14 @@ class ExecutorWithPriorityImpl : public virtual Executor {
} }
protected: protected:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_add(1, std::memory_order_relaxed); keepAliveCounter_.fetch_add(1, std::memory_order_relaxed);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
......
...@@ -124,12 +124,12 @@ class ManualExecutor : public DrivableExecutor, ...@@ -124,12 +124,12 @@ class ManualExecutor : public DrivableExecutor,
return funcs.size() + scheduled_funcs.size(); return funcs.size() + scheduled_funcs.size();
} }
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
keepAliveCount_.fetch_add(1, std::memory_order_relaxed); keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
if (keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel) == 1) { if (keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel) == 1) {
add([] {}); add([] {});
} }
......
...@@ -40,14 +40,14 @@ SerialExecutor::UniquePtr SerialExecutor::createUnique( ...@@ -40,14 +40,14 @@ SerialExecutor::UniquePtr SerialExecutor::createUnique(
return {executor, Deleter{std::move(parent)}}; return {executor, Deleter{std::move(parent)}};
} }
bool SerialExecutor::keepAliveAcquire() { bool SerialExecutor::keepAliveAcquire() noexcept {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_add(1, std::memory_order_relaxed); keepAliveCounter_.fetch_add(1, std::memory_order_relaxed);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
return true; return true;
} }
void SerialExecutor::keepAliveRelease() { void SerialExecutor::keepAliveRelease() noexcept {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
......
...@@ -101,9 +101,9 @@ class SerialExecutor : public SequencedExecutor { ...@@ -101,9 +101,9 @@ class SerialExecutor : public SequencedExecutor {
} }
protected: protected:
bool keepAliveAcquire() override; bool keepAliveAcquire() noexcept override;
void keepAliveRelease() override; void keepAliveRelease() noexcept override;
private: private:
struct Task { struct Task {
......
...@@ -196,14 +196,14 @@ uint8_t StrandExecutor::getNumPriorities() const { ...@@ -196,14 +196,14 @@ uint8_t StrandExecutor::getNumPriorities() const {
return parent_->getNumPriorities(); return parent_->getNumPriorities();
} }
bool StrandExecutor::keepAliveAcquire() { bool StrandExecutor::keepAliveAcquire() noexcept {
[[maybe_unused]] auto oldCount = [[maybe_unused]] auto oldCount =
refCount_.fetch_add(1, std::memory_order_relaxed); refCount_.fetch_add(1, std::memory_order_relaxed);
DCHECK(oldCount > 0); DCHECK(oldCount > 0);
return true; return true;
} }
void StrandExecutor::keepAliveRelease() { void StrandExecutor::keepAliveRelease() noexcept {
const auto oldCount = refCount_.fetch_sub(1, std::memory_order_acq_rel); const auto oldCount = refCount_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(oldCount > 0); DCHECK(oldCount > 0);
if (oldCount == 1) { if (oldCount == 1) {
......
...@@ -153,8 +153,8 @@ class StrandExecutor final : public SequencedExecutor { ...@@ -153,8 +153,8 @@ class StrandExecutor final : public SequencedExecutor {
uint8_t getNumPriorities() const override; uint8_t getNumPriorities() const override;
protected: protected:
bool keepAliveAcquire() override; bool keepAliveAcquire() noexcept override;
void keepAliveRelease() override; void keepAliveRelease() noexcept override;
private: private:
explicit StrandExecutor( explicit StrandExecutor(
......
...@@ -64,14 +64,14 @@ void TimekeeperScheduledExecutor::scheduleAt( ...@@ -64,14 +64,14 @@ void TimekeeperScheduledExecutor::scheduleAt(
} }
} }
bool TimekeeperScheduledExecutor::keepAliveAcquire() { bool TimekeeperScheduledExecutor::keepAliveAcquire() noexcept {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_add(1, std::memory_order_relaxed); keepAliveCounter_.fetch_add(1, std::memory_order_relaxed);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
return true; return true;
} }
void TimekeeperScheduledExecutor::keepAliveRelease() { void TimekeeperScheduledExecutor::keepAliveRelease() noexcept {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCounter_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(keepAliveCounter > 0); DCHECK(keepAliveCounter > 0);
......
...@@ -51,8 +51,8 @@ class TimekeeperScheduledExecutor : public ScheduledExecutor { ...@@ -51,8 +51,8 @@ class TimekeeperScheduledExecutor : public ScheduledExecutor {
override; override;
protected: protected:
bool keepAliveAcquire() override; bool keepAliveAcquire() noexcept override;
void keepAliveRelease() override; void keepAliveRelease() noexcept override;
private: private:
TimekeeperScheduledExecutor( TimekeeperScheduledExecutor(
......
...@@ -324,14 +324,14 @@ class BlockingWaitExecutor final : public folly::DrivableExecutor { ...@@ -324,14 +324,14 @@ class BlockingWaitExecutor final : public folly::DrivableExecutor {
} }
private: private:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCount = auto keepAliveCount =
keepAliveCount_.fetch_add(1, std::memory_order_relaxed); keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
DCHECK(keepAliveCount >= 0); DCHECK(keepAliveCount >= 0);
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCount = keepAliveCount_.load(std::memory_order_relaxed); auto keepAliveCount = keepAliveCount_.load(std::memory_order_relaxed);
do { do {
DCHECK(keepAliveCount > 0); DCHECK(keepAliveCount > 0);
......
...@@ -148,12 +148,12 @@ TEST_F(CoroTest, ExecutorKeepAlive) { ...@@ -148,12 +148,12 @@ TEST_F(CoroTest, ExecutorKeepAlive) {
} }
struct CountingExecutor : public ManualExecutor { struct CountingExecutor : public ManualExecutor {
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
++keepAliveCounter; ++keepAliveCounter;
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
--keepAliveCounter; --keepAliveCounter;
} }
......
...@@ -553,14 +553,14 @@ class WaitExecutor final : public folly::Executor { ...@@ -553,14 +553,14 @@ class WaitExecutor final : public folly::Executor {
private: private:
WaitExecutor() {} WaitExecutor() {}
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCount = auto keepAliveCount =
keepAliveCount_.fetch_add(1, std::memory_order_relaxed); keepAliveCount_.fetch_add(1, std::memory_order_relaxed);
DCHECK(keepAliveCount > 0); DCHECK(keepAliveCount > 0);
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCount = auto keepAliveCount =
keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel); keepAliveCount_.fetch_sub(1, std::memory_order_acq_rel);
DCHECK(keepAliveCount > 0); DCHECK(keepAliveCount > 0);
......
...@@ -815,7 +815,7 @@ class EventBase : public TimeoutManager, ...@@ -815,7 +815,7 @@ class EventBase : public TimeoutManager,
static std::unique_ptr<EventBaseBackendBase> getDefaultBackend(); static std::unique_ptr<EventBaseBackendBase> getDefaultBackend();
protected: protected:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
if (inRunningEventBaseThread()) { if (inRunningEventBaseThread()) {
loopKeepAliveCount_++; loopKeepAliveCount_++;
} else { } else {
...@@ -824,7 +824,7 @@ class EventBase : public TimeoutManager, ...@@ -824,7 +824,7 @@ class EventBase : public TimeoutManager,
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
if (!inRunningEventBaseThread()) { if (!inRunningEventBaseThread()) {
return add([this] { loopKeepAliveCount_--; }); return add([this] { loopKeepAliveCount_--; });
} }
......
...@@ -65,11 +65,11 @@ class ScopedEventBaseThread : public IOExecutor, public SequencedExecutor { ...@@ -65,11 +65,11 @@ class ScopedEventBaseThread : public IOExecutor, public SequencedExecutor {
} }
protected: protected:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
return getEventBase()->keepAliveAcquire(); return getEventBase()->keepAliveAcquire();
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
getEventBase()->keepAliveRelease(); getEventBase()->keepAliveRelease();
} }
......
...@@ -27,7 +27,7 @@ std::future<void> VirtualEventBase::destroy() { ...@@ -27,7 +27,7 @@ std::future<void> VirtualEventBase::destroy() {
return std::move(destroyFuture_); return std::move(destroyFuture_);
} }
void VirtualEventBase::destroyImpl() { void VirtualEventBase::destroyImpl() noexcept {
try { try {
{ {
// After destroyPromise_ is posted this object may be destroyed, so make // After destroyPromise_ is posted this object may be destroyed, so make
......
...@@ -124,7 +124,7 @@ class VirtualEventBase : public folly::TimeoutManager, ...@@ -124,7 +124,7 @@ class VirtualEventBase : public folly::TimeoutManager,
} }
protected: protected:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
if (evb_->inRunningEventBaseThread()) { if (evb_->inRunningEventBaseThread()) {
DCHECK(loopKeepAliveCount_ + loopKeepAliveCountAtomic_.load() > 0); DCHECK(loopKeepAliveCount_ + loopKeepAliveCountAtomic_.load() > 0);
...@@ -135,7 +135,7 @@ class VirtualEventBase : public folly::TimeoutManager, ...@@ -135,7 +135,7 @@ class VirtualEventBase : public folly::TimeoutManager,
return true; return true;
} }
void keepAliveReleaseEvb() { void keepAliveReleaseEvb() noexcept {
if (loopKeepAliveCountAtomic_.load()) { if (loopKeepAliveCountAtomic_.load()) {
loopKeepAliveCount_ += loopKeepAliveCountAtomic_.exchange(0); loopKeepAliveCount_ += loopKeepAliveCountAtomic_.exchange(0);
} }
...@@ -145,9 +145,9 @@ class VirtualEventBase : public folly::TimeoutManager, ...@@ -145,9 +145,9 @@ class VirtualEventBase : public folly::TimeoutManager,
} }
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
if (!evb_->inRunningEventBaseThread()) { if (!evb_->inRunningEventBaseThread()) {
evb_->add([=] { keepAliveReleaseEvb(); }); evb_->add([this] { keepAliveReleaseEvb(); });
return; return;
} }
...@@ -165,7 +165,7 @@ class VirtualEventBase : public folly::TimeoutManager, ...@@ -165,7 +165,7 @@ class VirtualEventBase : public folly::TimeoutManager,
} }
std::future<void> destroy(); std::future<void> destroy();
void destroyImpl(); void destroyImpl() noexcept;
using LoopCallbackList = EventBase::LoopCallback::List; using LoopCallbackList = EventBase::LoopCallback::List;
......
...@@ -69,7 +69,7 @@ class AsyncioExecutor : public DrivableExecutor, public SequencedExecutor { ...@@ -69,7 +69,7 @@ class AsyncioExecutor : public DrivableExecutor, public SequencedExecutor {
} }
protected: protected:
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
auto keepAliveCounter = auto keepAliveCounter =
keepAliveCounter_.fetch_add(1, std::memory_order_relaxed); keepAliveCounter_.fetch_add(1, std::memory_order_relaxed);
// We should never increment from 0 // We should never increment from 0
...@@ -77,7 +77,7 @@ class AsyncioExecutor : public DrivableExecutor, public SequencedExecutor { ...@@ -77,7 +77,7 @@ class AsyncioExecutor : public DrivableExecutor, public SequencedExecutor {
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
auto keepAliveCounter = --keepAliveCounter_; auto keepAliveCounter = --keepAliveCounter_;
DCHECK(keepAliveCounter >= 0); DCHECK(keepAliveCounter >= 0);
} }
......
...@@ -27,12 +27,12 @@ class KeepAliveTestExecutor : public Executor { ...@@ -27,12 +27,12 @@ class KeepAliveTestExecutor : public Executor {
// this executor does nothing // this executor does nothing
} }
bool keepAliveAcquire() override { bool keepAliveAcquire() noexcept override {
++refCount; ++refCount;
return true; return true;
} }
void keepAliveRelease() override { void keepAliveRelease() noexcept override {
--refCount; --refCount;
} }
......
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