[Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Matthew Gardiner]

Cool, let us know how you get on!
Matt

Shawn Best wrote:
> Thanks for the feedback guys.
>
> Studying the code, I had figured going with a straight int would in 
> practice be most efficient and not run into multi-threaded problems, 
> even if initially appearing a bit risky.  I will rework it to use a 
> std::condition_variable.  That will be more robust and readable.
>
> Shawn.
>
> On 7/29/2014 10:53 AM, Zachary Turner wrote:
>> Even better would be an std::condition_variable
>>
>>
>> On Mon, Jul 28, 2014 at 10:30 PM, Matthew Gardiner <mg11 at csr.com 
>> <mailto:mg11 at csr.com>> wrote:
>>
>>     Hi Shawn,
>>
>>     I use 64-bit linux and I see this issue a lot. It usually
>>     manifests itself as the prompt just not being printed (or perhaps
>>     it just gets overwritten) - regardless - I invoke a command, and
>>     I don't see an (lldb) prompt when I should. So I'm well pleased
>>     that you are looking at this!
>>
>>     Would it not be more robust to use a semaphore than usleep to
>>     synchronise the problematic threads?
>>
>>     Although I've not looked too deeply into this particular issue,
>>     whenever I've seen similar races, I found that it's almost
>>     impossible to pick the right value when using a sleep command. A
>>     semaphore, though, should always ensure the waiting thread will
>>     wake precisely.
>>
>>     I'd be happy to help to test such a fix.
>>
>>     Matt
>>
>>
>>     Shawn Best wrote:
>>
>>         Hi,
>>
>>         I have attached a patch which addresses 3 related race
>>         conditions that cause the command line (lldb) prompt to get
>>         displayed inappropriately and make it appear it is not
>>         working correctly.  This issue can be seen on linux and
>>         FreeBSD.  I can also artificailly induce the problem on OSX.
>>
>>         The issue happens when the command handler (in the main
>>         thread) issues a command such as run, step or continue.
>>          After the command finishes initiating its action, it returns
>>         up the call stack and goes back into the main command loop
>>         waiting for user input.  Simultaneously, as the inferior
>>         process starts up, the MonitorChildProcess thread picks up
>>         the change and posts to the PrivateEvent thread.
>>          HandePrivateEvent() then calls PushProcessIOHandler() which
>>         will disable the command IO handler and give the inferior
>>         control of the TTY.  To observe this on OSX, put a
>>
>>         usleep(100);
>>
>>         immediately prior the PushProcessIOHandler() in
>>         HandlePrivateEvent.
>>
>>
>>         My proposed solution is that after a 'run', 'step', or
>>         'continue' command, insert a synchronization point and wait
>>         until HandlePrivateEvent knows the inferior process is
>>         running and has pushed the IO handler.  One context switch
>>         (<100us) is usually all the time it takes on my machine.  As
>>         an additional safety, I have a timeout (currently 1ms) so it
>>         will never hang the main thread.
>>
>>         Any thoughts, or suggestions would be appreciated.
>>
>>         Regards,
>>         Shawn.
>>
>>
>>         To report this email as spam click here
>>         <https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==>.
>>
>>
>>
>>         _______________________________________________
>>         lldb-commits mailing list
>>         lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>>         http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>>
>>
>>
>>     Member of the CSR plc group of companies. CSR plc registered in
>>     England and Wales, registered number 4187346, registered office
>>     Churchill House, Cambridge Business Park, Cowley Road, Cambridge,
>>     CB4 0WZ, United Kingdom
>>     More information can be found at www.csr.com
>>     <http://www.csr.com>. Keep up to date with CSR on our technical
>>     blog, www.csr.com/blog <http://www.csr.com/blog>, CSR people
>>     blog, www.csr.com/people <http://www.csr.com/people>, YouTube,
>>     www.youtube.com/user/CSRplc <http://www.youtube.com/user/CSRplc>,
>>     Facebook, www.facebook.com/pages/CSR/191038434253534
>>     <http://www.facebook.com/pages/CSR/191038434253534>, or follow us
>>     on Twitter at www.twitter.com/CSR_plc
>>     <http://www.twitter.com/CSR_plc>.
>>     New for 2014, you can now access the wide range of products
>>     powered by aptX at www.aptx.com <http://www.aptx.com>.
>>     _______________________________________________
>>     lldb-commits mailing list
>>     lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>>
>