Commit a26a1a9e authored by Matthieu Martin's avatar Matthieu Martin Committed by Facebook Github Bot

Don't shallow copy default request context

Summary:
This is to be consistent with setContext, which doesn't call set/unset of the default context.
Main goal is to merge code in next diff.

I think the default context is never relevant, and should only be the current context in the internal of the async framework, in between executing coroutines.
We should probably enforce that request data is never set in the default context to guarantee correctness.

Reviewed By: djwatson

Differential Revision: D9119717

fbshipit-source-id: 8fa07520d506c02663f58f7616f1f390645997bc
parent 98fef9ad
......@@ -179,10 +179,10 @@ std::shared_ptr<RequestContext>& RequestContext::getStaticContext() {
/* static */ std::shared_ptr<RequestContext>
RequestContext::setShallowCopyContext() {
auto* parent = get();
auto& parent = getStaticContext();
auto child = std::make_shared<RequestContext>();
{
if (parent) {
auto parentLock = parent->state_.rlock();
auto childLock = child->state_.wlock();
childLock->callbackData_ = parentLock->callbackData_;
......@@ -193,21 +193,16 @@ RequestContext::setShallowCopyContext() {
}
// Do not use setContext to avoid global set/unset
std::swap(child, getStaticContext());
std::swap(child, parent);
return child;
}
/* static */ void RequestContext::unsetShallowCopyContext(
std::shared_ptr<RequestContext> ctx) {
// Do not use setContext to avoid set/unset
std::swap(ctx, getStaticContext());
// For readability
auto child = std::move(ctx);
// Handle the case where parent is actually default context
auto* parent = get();
std::shared_ptr<RequestContext> parent) {
auto& child = getStaticContext();
// Call set/unset for all request data that differs
{
if (parent && child) {
auto parentLock = parent->state_.rlock();
auto childLock = child->state_.rlock();
auto piter = parentLock->callbackData_.begin();
......@@ -233,7 +228,14 @@ RequestContext::setShallowCopyContext() {
++citer;
}
}
} else if (parent) {
parent->onSet();
} else if (child) {
child->onUnset();
}
// Do not use setContext to avoid set/unset
child = parent;
}
RequestContext* RequestContext::get() {
......
......@@ -123,14 +123,12 @@ TEST(RequestContext, RequestContextScopeGuard) {
TEST(RequestContext, defaultContext) {
// Don't create a top level guard
// Regression test for set/onset used to not work with the default context
setData(10);
{
RequestContextScopeGuard g1;
EXPECT_FALSE(hasData());
}
EXPECT_EQ(10, getData().data_);
// TODO: should be 2/1
EXPECT_EQ(1, getData().set_);
EXPECT_EQ(0, getData().unset_);
}
......@@ -257,8 +255,8 @@ TEST(RequestContext, ShallowCopyDefaultContext) {
EXPECT_EQ(789, getData().data_);
}
EXPECT_EQ(123, getData().data_);
EXPECT_EQ(2, getData().set_);
EXPECT_EQ(1, getData().unset_);
EXPECT_EQ(1, getData().set_);
EXPECT_EQ(0, getData().unset_);
}
TEST(RequestContext, ShallowCopyClear) {
......
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