Re: Threadsanitizer Detection on GM

--===============8652872672140363747==
Content-Type: multipart/alternative; boundary="000000000000dd98790581b6543d"

--000000000000dd98790581b6543d
Content-Type: text/plain; charset="UTF-8"

Hi Bob,

    Yes, I agree that this read/write race is mostly benign since this is
only used for monitoring
updates.
    There are other occurrences (e.g., the tarball I attached in a previous
mail has some)
such as "quantum" which also have similar issues, I can see that this value
seems to be
passed back and forward with "quantum_p" however I'm not aware of the
purpose.
    It's better to inspect them to see whether there are some potential
defects.
    Still I'd suggest "fixing" the potential data races as I can see the
overhead is mostly
neglectable and also may be better for maintenance during evolving. If
there are indeed
unnecessary to polish this, I think it's probably good to write some
comments or memo.

On Wed, Feb 13, 2019 at 1:38 AM Bob Friesenhahn <
[email protected]> wrote:

> On Wed, 13 Feb 2019, Hongxu Chen wrote:
> >
> > Yes, that is pass-by-value.
> > But the problem is that  there might also "(atomic) write row_count"
> > between these lines,
> > which causes the inconsistency.
> >
> > atomic write row_count at line:123
> > <write>
> > read  row_count at line:124 (QuantumTick)
> > <write>
> > read  row_count at line:125 (MagickMonitorFormatted)
> >
> > Therefore it's still possible that row_count at line:124 and line:125 are
> > different.
>
> Definitely the value can be different when reading a shared integer
> value which may be updated at any time by any thread.  This would be
> true if a lock is held around individual access, but not across the
> multiple accesses.  The updated value could be sampled and copied to a
> variable on the local thread stack (just like 'status' is copied to
> 'thread_status' at a point where there is an implicit flush) so it is
> consistent while used within the thread and there is no contention.
>
> QuantumTick() is a macro and refers to this value multiple times.  It
> is likely that the value used by the macro is in a register, or in L1
> cache, or optimized in interesting ways.  If the value changes while
> QuantumTick() is using it, it might produce a "tick" outside of its
> normal cadence.
>
> Regardless, this issue appears to be totally benign in terms of
> function of the software.  It is only there for the progress monitor,
> which is usually not active.
>
> Bob
>
>
> _______________________________________________
> Graphicsmagick-bugs mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs
>

--000000000000dd98790581b6543d
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div>Hi Bob,</div><div><br></div><div>=C2=
=A0 =C2=A0 Yes, I agree that this read/write race is mostly benign since th=
is is only used for monitoring</div><div>updates.</div><div>=C2=A0 =C2=A0 T=
here are other occurrences (e.g., the tarball I attached in a previous mail=
 has some)</div><div>such as &quot;quantum&quot; which also have similar is=
sues, I can see that this value seems to be</div><div>passed back and forwa=
rd with &quot;quantum_p&quot; however I&#39;m not aware of the purpose.</di=
v><div>=C2=A0 =C2=A0 It&#39;s better to inspect them to see whether there a=
re some potential defects.</div><div>=C2=A0 =C2=A0 Still I&#39;d suggest &q=
uot;fixing&quot; the potential data races as I can see the overhead is most=
ly=C2=A0</div><div>neglectable and also may be better for maintenance durin=
g evolving. If there are indeed</div><div>unnecessary to polish this, I thi=
nk it&#39;s probably good to write some comments or memo.</div><div dir=3D"=
ltr"><br></div><div dir=3D"ltr">On Wed, Feb 13, 2019 at 1:38 AM Bob Friesen=
hahn &lt;<a href=3D"mailto:[email protected]">[email protected]=
llas.tx.us</a>&gt; wrote:<br></div><div class=3D"gmail_quote"><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px soli=
d rgb(204,204,204);padding-left:1ex">On Wed, 13 Feb 2019, Hongxu Chen wrote=
:<br>
&gt;<br>
&gt; Yes, that is pass-by-value.<br>
&gt; But the problem is that=C2=A0 there might also &quot;(atomic) write ro=
w_count&quot;<br>
&gt; between these lines,<br>
&gt; which causes the inconsistency.<br>
&gt;<br>
&gt; atomic write row_count at line:123<br>
&gt; &lt;write&gt;<br>
&gt; read=C2=A0 row_count at line:124 (QuantumTick)<br>
&gt; &lt;write&gt;<br>
&gt; read=C2=A0 row_count at line:125 (MagickMonitorFormatted)<br>
&gt;<br>
&gt; Therefore it&#39;s still possible that row_count at line:124 and line:=
125 are<br>
&gt; different.<br>
<br>
Definitely the value can be different when reading a shared integer <br>
value which may be updated at any time by any thread.=C2=A0 This would be <=
br>
true if a lock is held around individual access, but not across the <br>
multiple accesses.=C2=A0 The updated value could be sampled and copied to a=
 <br>
variable on the local thread stack (just like &#39;status&#39; is copied to=
 <br>
&#39;thread_status&#39; at a point where there is an implicit flush) so it =
is <br>
consistent while used within the thread and there is no contention.<br>
<br>
QuantumTick() is a macro and refers to this value multiple times.=C2=A0 It =
<br>
is likely that the value used by the macro is in a register, or in L1 <br>
cache, or optimized in interesting ways.=C2=A0 If the value changes while <=
br>
QuantumTick() is using it, it might produce a &quot;tick&quot; outside of i=
ts <br>
normal cadence.<br>
<br>
Regardless, this issue appears to be totally benign in terms of <br>
function of the software.=C2=A0 It is only there for the progress monitor, =
<br>
which is usually not active.<br>
<br>
Bob<br>
<br>
<br>
_______________________________________________<br>
Graphicsmagick-bugs mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_bla=
nk">[email protected]</a><br>
<a href=3D"https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs=
" rel=3D"noreferrer" target=3D"_blank">https://lists.sourceforge.net/lists/=
listinfo/graphicsmagick-bugs</a><br>
</blockquote></div></div></div>

--000000000000dd98790581b6543d--


--===============8652872672140363747==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


--===============8652872672140363747==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Graphicsmagick-bugs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/graphicsmagick-bugs

--===============8652872672140363747==--