|
From: Julian S. <js...@ac...> - 2011-03-13 11:00:34
|
On Saturday, March 12, 2011, Bart Van Assche wrote:
> I have a quiz question for the fans of the --free-is-write=yes mode:
> why do Helgrind and DRD (and TSan probably too) report a race on
> drd/tests/annotate_smart_pointer ?
I think it's because the annotation of your release method -- "set" --
is not quite right. FWIW, I believe Konstantin's description of
marking up threadsafe refcounting has the same problem, but I can't
find the URL now. Anyway. The relevant bit of code is (after folding
in "s_enable_annotations == true"):
U_ANNOTATE_HAPPENS_BEFORE(m_count_ptr);
if (--(*m_count_ptr) == 0)
{
U_ANNOTATE_HAPPENS_AFTER(m_count_ptr);
delete m_ptr;
m_ptr = NULL;
delete m_count_ptr;
m_count_ptr = NULL;
}
}
and the --(*m_count_ptr) is just a call to __sync_sub_and_fetch.
Problem is that the h-b edge is created too early, and results in
the load and store from --(*m_count_ptr) being concurrent with
the delete. A version which I believe is correct is:
int new_count = --(*m_count_ptr);
U_ANNOTATE_HAPPENS_BEFORE(m_count_ptr);
if (new_count == 0)
{
U_ANNOTATE_HAPPENS_AFTER(m_count_ptr);
delete m_ptr;
m_ptr = NULL;
delete m_count_ptr;
m_count_ptr = NULL;
}
}
and if you change the code like that, the complaint goes away.
J
|