Commit b6916a3a authored by Igor Adamski's avatar Igor Adamski Committed by Facebook Github Bot

FunctionScheduler - set running_ when it has actually started running

Summary:
When start() throws because of reasons (in my case it was std::system_error("Resource temporarily unavailalble") coming from std::thread constructo)
and during the exception propagation we will have to destroy FunctionScheduler then
thread_.join() in shutdown will throw again.
This diff sets running_ after the thread is created.

Reviewed By: yfeldblum

Differential Revision: D4469816

fbshipit-source-id: cde54dfbf39f04d3ea9dfa02a65295f5440e5ea4
parent 84d872be
...@@ -318,8 +318,6 @@ bool FunctionScheduler::start() { ...@@ -318,8 +318,6 @@ bool FunctionScheduler::start() {
return false; return false;
} }
running_ = true;
VLOG(1) << "Starting FunctionScheduler with " << functions_.size() VLOG(1) << "Starting FunctionScheduler with " << functions_.size()
<< " functions."; << " functions.";
auto now = steady_clock::now(); auto now = steady_clock::now();
...@@ -334,6 +332,8 @@ bool FunctionScheduler::start() { ...@@ -334,6 +332,8 @@ bool FunctionScheduler::start() {
std::make_heap(functions_.begin(), functions_.end(), fnCmp_); std::make_heap(functions_.begin(), functions_.end(), fnCmp_);
thread_ = std::thread([&] { this->run(); }); thread_ = std::thread([&] { this->run(); });
running_ = true;
return true; return true;
} }
......
...@@ -13,16 +13,19 @@ ...@@ -13,16 +13,19 @@
* See the License for the specific language governing permissions and * See the License for the specific language governing permissions and
* limitations under the License. * limitations under the License.
*/ */
#include <folly/experimental/FunctionScheduler.h>
#include <algorithm> #include <algorithm>
#include <atomic> #include <atomic>
#include <cassert> #include <cassert>
#include <random> #include <random>
#include <folly/Random.h>
#include <folly/Random.h>
#include <folly/experimental/FunctionScheduler.h>
#include <folly/portability/GTest.h> #include <folly/portability/GTest.h>
#if defined(__linux__)
#include <dlfcn.h>
#endif
using namespace folly; using namespace folly;
using std::chrono::milliseconds; using std::chrono::milliseconds;
...@@ -488,6 +491,60 @@ TEST(FunctionScheduler, cancelFunctionAndWait) { ...@@ -488,6 +491,60 @@ TEST(FunctionScheduler, cancelFunctionAndWait) {
fs.shutdown(); fs.shutdown();
} }
#if defined(__linux__)
namespace {
/**
* A helper class that forces our pthread_create() wrapper to fail when
* an PThreadCreateFailure object exists.
*/
class PThreadCreateFailure {
public:
PThreadCreateFailure() {
++forceFailure_;
}
~PThreadCreateFailure() {
--forceFailure_;
}
static bool shouldFail() {
return forceFailure_ > 0;
}
private:
static std::atomic<int> forceFailure_;
};
std::atomic<int> PThreadCreateFailure::forceFailure_{0};
} // unnamed namespce
// Replace the system pthread_create() function with our own stub, so we can
// trigger failures in the StartThrows() test.
extern "C" int pthread_create(
pthread_t* thread,
const pthread_attr_t* attr,
void* (*start_routine)(void*),
void* arg) {
static const auto realFunction = reinterpret_cast<decltype(&pthread_create)>(
dlsym(RTLD_NEXT, "pthread_create"));
// For sanity, make sure we didn't find ourself,
// since that would cause infinite recursion.
CHECK_NE(realFunction, pthread_create);
if (PThreadCreateFailure::shouldFail()) {
errno = EINVAL;
return -1;
}
return realFunction(thread, attr, start_routine, arg);
}
TEST(FunctionScheduler, StartThrows) {
FunctionScheduler fs;
PThreadCreateFailure fail;
EXPECT_ANY_THROW(fs.start());
EXPECT_NO_THROW(fs.shutdown());
}
#endif
TEST(FunctionScheduler, cancelAllFunctionsAndWait) { TEST(FunctionScheduler, cancelAllFunctionsAndWait) {
int total = 0; int total = 0;
FunctionScheduler fs; FunctionScheduler fs;
......
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