Skip to content

Commit

Permalink
Extend FuncRunner to include ExecutionObserver list callbacks
Browse files Browse the repository at this point in the history
Summary:
Currently we have following code:

```
if (res != 0 && !queue_->empty()) {
    ExecutionObserverScopeGuard guard(&executionObserverList_, queue_.get());
    queue_->execute();
}
```

Which is wrong because queue_->execute() will execute all the callbacks inline, instead ExecutionObserver should be applied to individual callbacks.

ot suggested to add those changes to FuncRunner.

Reviewed By: yfeldblum, ot

Differential Revision: D53736878

fbshipit-source-id: 44d7e83daad06eee262a877afc0a0f11ba4f2280
  • Loading branch information
Giorgi Papakerashvili authored and facebook-github-bot committed Feb 26, 2024
1 parent ca0bd66 commit 7ca2d7e
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions folly/io/async/EventBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,16 @@ namespace folly {

class EventBase::FuncRunner {
public:
void operator()(Func func) noexcept { func(); }
explicit FuncRunner(EventBase& eventBase) : eventBase_(eventBase) {}
void operator()(Func&& func) noexcept {
ExecutionObserverScopeGuard guard(
&eventBase_.getExecutionObserverList(), &func);

func();
}

private:
EventBase& eventBase_;
};

class EventBase::ThreadIdCollector : public WorkerProvider {
Expand Down Expand Up @@ -592,7 +601,6 @@ EventBase::LoopStatus EventBase::loopMain(int flags, LoopOptions options) {
// loop callback scheduled by execute(), so if there is an enqueue after the
// empty check here the queue's event will eventually be active.
if (res != 0 && !queue_->empty()) {
ExecutionObserverScopeGuard guard(&executionObserverList_, queue_.get());
queue_->execute();
}

Expand Down Expand Up @@ -947,8 +955,8 @@ bool EventBase::runLoopCallbacks() {

void EventBase::initNotificationQueue() {
// Infinite size queue
queue_ =
std::make_unique<EventBaseAtomicNotificationQueue<Func, FuncRunner>>();
queue_ = std::make_unique<EventBaseAtomicNotificationQueue<Func, FuncRunner>>(
FuncRunner{*this});

// Mark this as an internal event, so event_base_loop() will return if
// there are no other events besides this one installed.
Expand Down

0 comments on commit 7ca2d7e

Please sign in to comment.