[Lldb-commits] [PATCH] D19122: LLDB: Fixed race condition on timeout when stopping private state thread

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Thu Apr 14 15:57:55 PDT 2016


At the Command & SB API level, we restrict access to the process from multiple threads with the run lock.  But internally it is currently more of a gentleman's agreement not to do this.  

I don't think it would ever be useful to try to make calling functions in the target (which is the job of RunThreadPlan) work concurrently.  Rather we should have some higher level permission baton that you have to have to do anything that would run the target, and then let only one thread at a time do this work.  That would make it impossible for RunThreadPlan to get run on multiple threads for the same process.  So while it wouldn't hurt to protect the m_private_state_thread variable with locks, that's not really the right level to do this, and if you ever let two threads try to run function calls in the target simultaneously, racy-ness in the m_private_state_thread would be the least of your problems...

Jim


> On Apr 14, 2016, at 3:44 PM, Cameron <cameron at moodycamel.com> wrote:
> 
> cameron314 added a comment.
> 
> I read the same docs :D This is the important part:
> 
>> If multiple threads of execution access the same shared_ptr without synchronization and any of those accesses uses a non-const member function of shared_ptr then a data race will occur; the shared_ptr overloads of atomic functions can be used to prevent the data race.
> 
> 
> Copying the shared pointer and accessing its pointee through it is thread safe (copying a wrapper object technically isn't, but it will boil down to the same thing, so let's leave that aside). But copying it while the pointer is being reassigned (`operator=`) on a different thread is //not// thread safe.
> 
> To answer your question, yes, absolutely, we can just remove that one call to Reset. But if the copy is still necessary, that means a lot of existing code (including that copy) is also racy and should be reviewed (though this would make sense to do in a separate patch).
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D19122
> 
> 
> 



More information about the lldb-commits mailing list