Commit 2215f6c0 authored by Marc Horowitz's avatar Marc Horowitz Committed by Sara Golemon

Don't check for thread running in terminateLoopSoon()

Summary:
Consider this use case:
thread 1:
make an evb
create and start thread 2
do some work
evb->terminateLoopSoon()
join thread 2 with a timeout
thread 2:
do some initialization
evb->loop()
Normally, this all works fine.  However, if the work thread 1 has to do is sufficiently small, and no external input occurs (one of the thing the evb is doing is listening on a socket), then sometimes, terminateLoopSoon() happens before loop() is called (or at least, before loopThread_.store() happens). isRunning() in terminateLoopSoon() is false, and so stop_ is never set, and event_base_loopbreak() is never called. The join times out, and thread 2 is still running.  Removing the isRunning() check gives the desired behavior.

Test Plan:
In my ad hoc tests, this fix caused my threads to exit when
I wanted them to exit in a situation like the one above.

Reviewed By: andrewcox@fb.com

FB internal diff: D1455885

Tasks: 2057547
parent 9633f475
...@@ -393,10 +393,6 @@ bool EventBase::bumpHandlingTime() { ...@@ -393,10 +393,6 @@ bool EventBase::bumpHandlingTime() {
void EventBase::terminateLoopSoon() { void EventBase::terminateLoopSoon() {
VLOG(5) << "EventBase(): Received terminateLoopSoon() command."; VLOG(5) << "EventBase(): Received terminateLoopSoon() command.";
if (!isRunning()) {
return;
}
// Set stop to true, so the event loop will know to exit. // Set stop to true, so the event loop will know to exit.
// TODO: We should really use an atomic operation here with a release // TODO: We should really use an atomic operation here with a release
// barrier. // barrier.
......
...@@ -184,7 +184,11 @@ class EventBase : private boost::noncopyable, public TimeoutManager { ...@@ -184,7 +184,11 @@ class EventBase : private boost::noncopyable, public TimeoutManager {
* to wake up and return in the EventBase loop thread. terminateLoopSoon() * to wake up and return in the EventBase loop thread. terminateLoopSoon()
* may also be called from the loop thread itself (for example, a * may also be called from the loop thread itself (for example, a
* EventHandler or AsyncTimeout callback may call terminateLoopSoon() to * EventHandler or AsyncTimeout callback may call terminateLoopSoon() to
* cause the loop to exit after the callback returns.) * cause the loop to exit after the callback returns.) If the loop is not
* running, this will cause the next call to loop to terminate soon after
* starting. If a loop runs out of work (and so terminates on its own)
* concurrently with a call to terminateLoopSoon(), this may cause a race
* condition.
* *
* Note that the caller is responsible for ensuring that cleanup of all event * Note that the caller is responsible for ensuring that cleanup of all event
* callbacks occurs properly. Since terminateLoopSoon() causes the loop to * callbacks occurs properly. Since terminateLoopSoon() causes the loop to
......
/*
* Copyright 2014 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <unistd.h>
#include <thread>
#include <gtest/gtest.h>
#include <folly/io/async/EventBase.h>
#include <folly/io/async/EventHandler.h>
using namespace folly;
class PipeHandler : public EventHandler {
public:
PipeHandler(EventBase* eventBase, int fd)
: EventHandler(eventBase, fd) {}
void handlerReady(uint16_t events) noexcept {
abort();
}
};
TEST(EventBase, StopBeforeLoop) {
EventBase evb;
// Give the evb something to do.
int p[2];
ASSERT_EQ(0, pipe(p));
PipeHandler handler(&evb, p[0]);
handler.registerHandler(EventHandler::READ);
// It's definitely not running yet
evb.terminateLoopSoon();
// let it run, it should exit quickly.
std::thread t([&] { evb.loop(); });
t.join();
handler.unregisterHandler();
close(p[0]);
close(p[1]);
SUCCEED();
}
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