Commit c0f5b564 authored by Misha Shneerson's avatar Misha Shneerson Committed by Facebook Github Bot

snapshot rootID at the same time as we snapshot callstacks

Summary:
D19417574 added support for snapshotting stack traces for threads our requests are running on. The problem with this approach was that we are racing - stack trace may be recorded AFTER requests has already left the thread. Hence we might be presenting incorrect and confusing picture to the users.

Here we make sure that request root id and stack trace are recorded together, and there is no race.

Because we are using interrupts to get on the thread, sometimes we might not be able to record the information using this approach. We then fallback to the previous way of associating threads and requests (by walking TLS entries).

(Note: this ignores all push blocking failures!)

Reviewed By: andriigrynenko

Differential Revision: D19445100

fbshipit-source-id: 4311f4e521a48852bf99aede03814de0941c7814
parent 00647a8d
......@@ -463,6 +463,9 @@ RequestContext::RequestContext()
: useHazptr_(FLAGS_folly_reqctx_use_hazptr),
rootId_(reinterpret_cast<intptr_t>(this)) {}
RequestContext::RequestContext(intptr_t rootid)
: useHazptr_(FLAGS_folly_reqctx_use_hazptr), rootId_(rootid) {}
RequestContext::RequestContext(const RequestContext& ctx, RootTag)
: RequestContext(ctx) {
rootId_ = reinterpret_cast<intptr_t>(this);
......@@ -798,7 +801,7 @@ RequestContext::setShallowCopyContext() {
RequestContext* RequestContext::get() {
auto& context = getStaticContext().first;
if (!context) {
static RequestContext defaultContext;
static RequestContext defaultContext(0);
return std::addressof(defaultContext);
}
return context.get();
......
......@@ -270,6 +270,7 @@ class RequestContext {
public:
RequestContext(const RequestContext& ctx, RootTag tag);
RequestContext(const RequestContext& ctx, ChildTag tag);
explicit RequestContext(intptr_t rootId);
using StaticContext = std::pair<std::shared_ptr<RequestContext>, intptr_t>;
private:
......
......@@ -88,6 +88,8 @@ TEST_F(RequestContextTest, SimpleTest) {
// There should always be a default context with get()
EXPECT_TRUE(RequestContext::get() != nullptr);
// but fallback context should have rootid set to 0
EXPECT_EQ(RequestContext::get()->getRootId(), 0);
// but not with saveContext()
EXPECT_EQ(RequestContext::saveContext(), nullptr);
......@@ -97,6 +99,7 @@ TEST_F(RequestContextTest, SimpleTest) {
EXPECT_EQ(1, rootids.size());
EXPECT_EQ(RequestContext::get()->getRootId(), rootids[0]);
EXPECT_EQ(reinterpret_cast<intptr_t>(RequestContext::get()), rootids[0]);
EXPECT_NE(RequestContext::get()->getRootId(), 0);
RequestContext::create();
EXPECT_NE(RequestContext::saveContext(), nullptr);
EXPECT_NE(RequestContext::get()->getRootId(), rootids[0]);
......
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