Commit 291c761f authored by Lara Lu's avatar Lara Lu Committed by Facebook Github Bot

add copy constructor for KeepAlive

Summary:
There is a need for KeepAlive copy constructor from various users. Adding them after chatting with Andrii.
Background: move is preferred to copy as the latter does an atomic counter increment. `copy()` was introduced but there is a need for more user-friendly way of copying.
TODO: maybe get rid of `copy()` and its usages?

Reviewed By: andriigrynenko

Differential Revision: D13864057

fbshipit-source-id: 38b2e45285566a3c2dba01706e82f421285b6bae
parent ff841386
......@@ -61,6 +61,9 @@ class Executor {
KeepAlive(KeepAlive&& other) noexcept
: executorAndDummyFlag_(exchange(other.executorAndDummyFlag_, 0)) {}
KeepAlive(const KeepAlive& other) noexcept
: KeepAlive(getKeepAliveToken(other.get())) {}
template <
typename OtherExecutor,
typename = typename std::enable_if<
......@@ -70,6 +73,17 @@ class Executor {
other.executorAndDummyFlag_ = 0;
}
template <
typename OtherExecutor,
typename = typename std::enable_if<
std::is_convertible<OtherExecutor*, ExecutorT*>::value>::type>
/* implicit */ KeepAlive(const KeepAlive<OtherExecutor>& other) noexcept
: KeepAlive(getKeepAliveToken(other.get())) {}
/* implicit */ KeepAlive(ExecutorT* executor) {
*this = getKeepAliveToken(executor);
}
KeepAlive& operator=(KeepAlive&& other) {
reset();
executorAndDummyFlag_ = exchange(other.executorAndDummyFlag_, 0);
......@@ -84,6 +98,14 @@ class Executor {
return *this = KeepAlive(std::move(other));
}
template <
typename OtherExecutor,
typename = typename std::enable_if<
std::is_convertible<OtherExecutor*, ExecutorT*>::value>::type>
KeepAlive& operator=(const KeepAlive<OtherExecutor>& other) {
return *this = KeepAlive(other);
}
void reset() {
if (Executor* executor = get()) {
if (exchange(executorAndDummyFlag_, 0) & kDummyFlag) {
......
......@@ -52,7 +52,7 @@ TEST(ExecutorTest, KeepAliveBasic) {
EXPECT_EQ(0, exec.refCount);
}
TEST(ExecutorTest, KeepAliveMove) {
TEST(ExecutorTest, KeepAliveMoveConstructor) {
KeepAliveTestExecutor exec;
{
......@@ -61,11 +61,98 @@ TEST(ExecutorTest, KeepAliveMove) {
EXPECT_EQ(&exec, ka.get());
EXPECT_EQ(1, exec.refCount);
auto ka2 = std::move(ka);
// member move constructor
Executor::KeepAlive<KeepAliveTestExecutor> ka2(std::move(ka));
EXPECT_FALSE(ka);
EXPECT_TRUE(ka2);
EXPECT_EQ(&exec, ka2.get());
EXPECT_EQ(1, exec.refCount);
// template move constructor
Executor::KeepAlive<Executor> ka3(std::move(ka2));
EXPECT_FALSE(ka2);
EXPECT_TRUE(ka3);
EXPECT_EQ(&exec, ka3.get());
EXPECT_EQ(1, exec.refCount);
}
EXPECT_EQ(0, exec.refCount);
}
TEST(ExecutorTest, KeepAliveCopyConstructor) {
KeepAliveTestExecutor exec;
{
auto ka = getKeepAliveToken(exec);
EXPECT_TRUE(ka);
EXPECT_EQ(&exec, ka.get());
EXPECT_EQ(1, exec.refCount);
// member copy constructor
Executor::KeepAlive<KeepAliveTestExecutor> ka2(ka);
EXPECT_TRUE(ka);
EXPECT_TRUE(ka2);
EXPECT_EQ(&exec, ka2.get());
EXPECT_EQ(2, exec.refCount);
// template copy constructor
Executor::KeepAlive<Executor> ka3(ka);
EXPECT_TRUE(ka);
EXPECT_TRUE(ka3);
EXPECT_EQ(&exec, ka3.get());
EXPECT_EQ(3, exec.refCount);
}
EXPECT_EQ(0, exec.refCount);
}
TEST(ExecutorTest, KeepAliveImplicitConstructorFromRawPtr) {
KeepAliveTestExecutor exec;
{
auto myFunc = [&exec](Executor::KeepAlive<> /*ka*/) mutable {
EXPECT_EQ(1, exec.refCount);
};
myFunc(&exec);
}
EXPECT_EQ(0, exec.refCount);
}
TEST(ExecutorTest, KeepAliveMoveAssignment) {
KeepAliveTestExecutor exec;
{
auto ka = getKeepAliveToken(exec);
EXPECT_TRUE(ka);
EXPECT_EQ(&exec, ka.get());
EXPECT_EQ(1, exec.refCount);
Executor::KeepAlive<> ka2;
ka2 = std::move(ka);
EXPECT_FALSE(ka);
EXPECT_TRUE(ka2);
EXPECT_EQ(&exec, ka2.get());
EXPECT_EQ(1, exec.refCount);
}
EXPECT_EQ(0, exec.refCount);
}
TEST(ExecutorTest, KeepAliveCopyAssignment) {
KeepAliveTestExecutor exec;
{
auto ka = getKeepAliveToken(exec);
EXPECT_TRUE(ka);
EXPECT_EQ(&exec, ka.get());
EXPECT_EQ(1, exec.refCount);
auto ka2 = ka;
EXPECT_TRUE(ka);
EXPECT_TRUE(ka2);
EXPECT_EQ(&exec, ka2.get());
EXPECT_EQ(2, exec.refCount);
}
EXPECT_EQ(0, exec.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