Commit be983813 authored by Hannes Roth's avatar Hannes Roth Committed by Sara Golemon

(Wangle) Fix bug with CrappyExecutors, and bad PriorityExecutor

Summary: 1) We forgot to `--attached_` if `x` throws an exception
2) `PriorityExecutor` didn't execute `Func`, causing leaks in the test (not a bug in Futures)
3) I moved up the initialization for an empty `Core` into the constructor to make it easier to see

Reviewed By: @jsedgwick

Differential Revision: D2187343
parent 2c111cff
...@@ -78,7 +78,7 @@ class Core { ...@@ -78,7 +78,7 @@ class Core {
/// This must be heap-constructed. There's probably a way to enforce that in /// This must be heap-constructed. There's probably a way to enforce that in
/// code but since this is just internal detail code and I don't know how /// code but since this is just internal detail code and I don't know how
/// off-hand, I'm punting. /// off-hand, I'm punting.
Core() {} Core() : result_(), fsm_(State::Start), attached_(2) {}
explicit Core(Try<T>&& t) explicit Core(Try<T>&& t)
: result_(std::move(t)), : result_(std::move(t)),
...@@ -342,6 +342,7 @@ class Core { ...@@ -342,6 +342,7 @@ class Core {
}, priority); }, priority);
} }
} catch (...) { } catch (...) {
--attached_; // Account for extra ++attached_ before try
result_ = Try<T>(exception_wrapper(std::current_exception())); result_ = Try<T>(exception_wrapper(std::current_exception()));
callback_(std::move(*result_)); callback_(std::move(*result_));
} }
...@@ -364,10 +365,10 @@ class Core { ...@@ -364,10 +365,10 @@ class Core {
char lambdaBuf_[lambdaBufSize]; char lambdaBuf_[lambdaBufSize];
// place result_ next to increase the likelihood that the value will be // place result_ next to increase the likelihood that the value will be
// contained entirely in one cache line // contained entirely in one cache line
folly::Optional<Try<T>> result_ {}; folly::Optional<Try<T>> result_;
std::function<void(Try<T>&&)> callback_ {nullptr}; std::function<void(Try<T>&&)> callback_ {nullptr};
FSM<State> fsm_ {State::Start}; FSM<State> fsm_;
std::atomic<unsigned char> attached_ {2}; std::atomic<unsigned char> attached_;
std::atomic<bool> active_ {true}; std::atomic<bool> active_ {true};
std::atomic<bool> interruptHandlerSet_ {false}; std::atomic<bool> interruptHandlerSet_ {false};
folly::MicroSpinLock interruptLock_ {0}; folly::MicroSpinLock interruptLock_ {0};
......
...@@ -191,7 +191,7 @@ TEST(Via, chain3) { ...@@ -191,7 +191,7 @@ TEST(Via, chain3) {
struct PriorityExecutor : public Executor { struct PriorityExecutor : public Executor {
void add(Func f) override {} void add(Func f) override {}
void addWithPriority(Func, int8_t priority) override { void addWithPriority(Func f, int8_t priority) override {
int mid = getNumPriorities() / 2; int mid = getNumPriorities() / 2;
int p = priority < 0 ? int p = priority < 0 ?
std::max(0, mid + priority) : std::max(0, mid + priority) :
...@@ -205,6 +205,7 @@ struct PriorityExecutor : public Executor { ...@@ -205,6 +205,7 @@ struct PriorityExecutor : public Executor {
} else if (p == 2) { } else if (p == 2) {
count2++; count2++;
} }
f();
} }
uint8_t getNumPriorities() const override { uint8_t getNumPriorities() const override {
......
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