Experimental Physics and
Industrial Control System

Michael Davidsaver via Core-talk <core-talk at aps.anl.gov> · Mon, 9 Mar 2020 09:28:03 -0700

On 3/9/20 3:59 AM, Ralph Lange via Core-talk wrote:
> On Mon, 9 Mar 2020 at 06:47, Michael Davidsaver via Core-talk <core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov>> wrote:
> 
>     In following up on a strange CI test failure I noticed during the recent codeathon[1]
>     I realized a mistake I made in adding epicsThreadMustJoin() [2].  This change
>     introduced a reference counter to struct epicsThreadOSD.  The bug is in
>     (conditionally) incrementing the ref counter after pthread_create().
>     This allows a short-lived thread which attempts to self-join to race for a double free().
>     And it happens that epicsThreadTest does this.
> 
>     The fix is I think straight forward [3].  I'm wondering how severe this issue should be considered?
>     It's a race which can cause a crash at runtime.  However, the circumstances seem not so common.
> 
> 
> Well spotted!
> 
> Given that "a short-lived thread which attempts to self-join" is not a common situation, especially as this API was added very recently, I would suggest creating a regular LP ticket and fix the bug with the next release. Maybe step up the ticket priority to indicate that the issue might cause crashes.

https://bugs.launchpad.net/bugs/1866651

The caveat to this being a recently added API is that I wired
it into the epicsThread class.  Though I also don't think this
situation is likely to occur in the wild with c++ either.

I used the same design for the WIN32 version of epicsThreadMustJoin()
which will need an analogous change.  appveyor is churning through
this now.

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System