Skip to content

Commit

Permalink
Detect/remove stale threadinfo in clone children
Browse files Browse the repository at this point in the history
When parsing clone exit events, specifically for the child half of a
clone and when in a container, detect and potentially remove stale
threadinfo state for the child thread.

Generally the client have of a clone is responsible for creating the
thread state for the new thread, as long as the parent is in a
container. See the parent half of the "if(childtid == 0)" statement. We
simply need to verify in the child half that the parent really was in a
container.

You can find the parent thread id from the syscall return information,
which is moved up from below. Look up the parent thread and see if its
vtid/vpid differs from tid/pid. If so, any existing thread state must be
stale and remove it. Note that you can't use
evt->m_tinfo->get_parent_thread() directly, as that comes from the
existing potentially stale threadinfo.

This fixes #664.
  • Loading branch information
mstemm committed Nov 7, 2016
1 parent 8763adc commit 03d45a4
Showing 1 changed file with 53 additions and 23 deletions.
76 changes: 53 additions & 23 deletions userspace/libsinsp/parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,58 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)
{
//
// clone() returns 0 in the child.
//

int64_t parenttid;

//
// Check if this is a process or a new thread
//
if(flags & PPM_CL_CLONE_THREAD)
{
//
// This is a thread, the parent tid is the pid
//
parinfo = evt->get_param(4);
ASSERT(parinfo->m_len == sizeof(int64_t));
parenttid = *(int64_t *)parinfo->m_val;
}
else
{
//
// This is not a thread, the parent tid is ptid
//
parinfo = evt->get_param(5);
ASSERT(parinfo->m_len == sizeof(int64_t));
parenttid = *(int64_t *)parinfo->m_val;
}

//
// If the threadinfo in the event exists, and we're in
// a container, the threadinfo in the event must be
// stale (e.g. from a prior process with the same
// tid), because only the child side of a clone
// creates the threadinfo for the child. Clear and
// remove the old threadinfo.
//
if(evt->m_tinfo && in_container)
{
// See if the parent thread is in a
// container. If it is, the parent thread
// did *not* create the thread for this child,
// and any existing thread state must be
// stale.

sinsp_threadinfo* ptinfo = m_inspector->get_thread(parenttid, false, true);


if(ptinfo && ptinfo->m_tid != ptinfo->m_vtid)
{
m_inspector->remove_thread(tid, true);
evt->m_tinfo = NULL;
}
}

// Validate that the child thread info has actually been created.
//
if(!evt->m_tinfo)
Expand All @@ -969,40 +1021,18 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)
//
childtid = tid;

//
// Check if this is a process or a new thread
//
if(flags & PPM_CL_CLONE_THREAD)
{
//
// This is a thread, the parent tid is the pid
//
parinfo = evt->get_param(4);
ASSERT(parinfo->m_len == sizeof(int64_t));
tid = *(int64_t *)parinfo->m_val;
}
else
{
//
// This is not a thread, the parent tid is ptid
//
parinfo = evt->get_param(5);
ASSERT(parinfo->m_len == sizeof(int64_t));
tid = *(int64_t *)parinfo->m_val;
}
tid = parenttid;

//
// Keep going and add the event with the standard code below
//
}
else
{
//
// We are in the child's clone. If we are in a container, make
// sure the vtid/vpid are reflected because the father was maybe
// running outside the container so created the child thread without
// knowing the internal vtid/vpid
//
if(in_container)
{
evt->m_tinfo->m_vtid = vtid;
Expand Down

0 comments on commit 03d45a4

Please sign in to comment.